Hi Ross, Sorry for the delay, so tired and busy here! Ross Gammon <ross@the-gammons.net> writes: > Please note, I was not able to do a test installation to check hydrogen > is working. I hope you can confirm that. > I can confirm it works for everything I use it for, but unfortunately I don't have any midi gear to test midi-related functionality. > 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. > Thank you for spotting this. I've activated a build hook to catch cases like this in the future. Fixed, and fixed an assumption I had made about the translations. > Minor things: > 1. We could enable Salsa CI by adding a debian/salsa-ci.yml file. We could, but I don't think Hydrogen's test is significant enough to make it worthwhile to spin up an instance every time someone pushes to the repo (cost of resources, and cost of carbon footprint). > 2. I think the copyright-hints & check files can be removed as they were > used by cdbs? Done, queued for the source-only -2 upload > 3. The github issues page could be added to the upstream metadata > file. Is this user facing? I have been specifically omitting this from my packages, because I worry that it will make it more convenient for the user to click and report upstream, despite "Don't file bugs upstream" (https://www.debian.org/Bugs/Reporting). > 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. Yeah, it's confusing to me too, and yes, ought to be solved upstream. The png (erroneously named as a bmp) is used internally for something, and I'm not sure using the svg in that context would succeed, but it seems like it ought to be. The SVGs we install are: usr/share/hydrogen/data/img/gray/h2-icon.svg usr/share/hydrogen/data/img/gray/icon.svg usr/share/hydrogen/data/img/gray/warning.svg usr/share/icons/hicolor/scalable/apps/org.hydrogenmusic.Hydrogen.svg > 5. Patch 2001 should probably be tagged as "Forwarded: not-needed" as it > seems Debian specific. Done. Queued for -2. > 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? Possibly. I will investigate and plan to solve this for -2. > 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)? > I believe I cut the bin:lib-pkg ones, but I see I missed the docs ones. Fixed. > 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. I considered enabling new features, but chose to change the package as little as possible, because we're so late in the bullseye cycle. I'm of course open to enabling them if someone else will thoroughly test the features. > Erich also switched the jack dependency to jack2. When building with libjack-dev, debhelper should indirectly generate a dependency on libjack-jackd2-0 | libjack-0.125. My rational for not switching is that I'm aware of users of older Firewire interfaces who prefer (or need) jack and not jack2. I also asked #debian-multimedia for confirmation that sticking with jack was consistent with team policy and received a firm statement to not switch to building against jack2-dev. > 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? > If I remember correctly this was one of the previous maintainers' decisions, and I think it had to with putting everything is arch:all into the -data package--personally I think that's a reasonable design decision. bin:hydrogen Depends on bin:hydrogen-data, and bin:hydrogen-data Recommends bin:hydrogen, so they'll almost always both be installed. Do you know if appdata files should always go in the same package as the executable? Thanks again for taking the time to review and sponsor! Nicholas
Attachment:
signature.asc
Description: PGP signature