[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#976113: RFS Review for Hydrogen



Hi Nicholas,

Good stuff. I think we are just about ready to upload. Here are the
comments from my review.

Please note, I was not able to do a test installation to check hydrogen
is working. I hope you can confirm that.

Blocking upload:
1. The package fails to build twice in a row (I used the --twice option
in pdebuild). It appears some translation files are not being cleaned
after the first build.

Minor things:
1. We could enable Salsa CI by adding a debian/salsa-ci.yml file.
2. I think the copyright-hints & check files can be removed as they were
used by cdbs?
3. The github issues page could be added to the upstream metadata file.
4. I am not sure what is going on with the icon. There is a lot going on
in d/rules and also a patch. This is probably something that should be
sorted out upstream. As a minimum we should probably forward our patch
upstream or tag the patch as "Forwarded: not-needed" if it is not an
upstream issue. I note that in Ubuntu, they install an svg into the
pixmaps folder.
5. Patch 2001 should probably be tagged as "Forwarded: not-needed" as it
seems Debian specific.
6. I see that even with all hardening options enabled in d/rules, we
still get the "hardening no fortify" lintian error. Are there some flags
not being passed to the build system?
7. I see the large number of commits to add library packages and upload
pre-release versions has resulted in some churn in the changelog. I
think some entries can be removed now that we have agreed to upload a
simpler structure and because it is a proper release (e.g. disabling the
documentation)?

Suggestions/Considerations:
1. It is worth taking a look at the version of hydrogen uploaded in
Ubuntu by Erich Eickmeyer. By adding ladspa-sdk and libtar as build
dependencies (and a patch), it appears that the hydrogen builds with
extra options. Erich also switched the jack dependency to jack2.
2. I see we have added a lintian override for the desktop and appdata
files not being in the same package as the executable. Is there a reason
for not just installing the desktop/appdata files with the executable
instead of in -data?

-- 
Regards,

Ross Gammon
FBEE 0190 904F 1EA0 BA6A  300E 53FE 7BBD A689 10FC

Attachment: signature.asc
Description: OpenPGP digital signature


Reply to: