Code review comment for ~lvoytek/ubuntu/+source/open-isns:open-isns-101-update-jammy

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Tests, Builds, Bug references, Changelog, ... all that LGTM.

I checked (a common case on new upstream versions) if we'd need to bump d/copyright.
But interestingly there was no bump in the upstream source v0.100->v0.101, so that should be fine as is.

One thing that I saw was that upstream added a man-page and that is great, but we usually want to ensure it is installed. The new file is doc/isnssetup.8.
I see the build does
  /usr/bin/install -c -m 644 ./doc/isnssetup.8 /<<PKGBUILDDIR>>/debian/tmp/usr/share/man/man8

But it isn't picked up by the packaging and not in the binary package.
But that led me to this trail - and after a check I found a few things to be missing:

1. the isnssetup helpe rscript, directly available in the source probably a good candidate for either open-isns-utils in /usr/sbin/ along the other admin tools, or at least as example in /usr/share/doc/open-isns-utils/. I guess /usr/sbin along the others is more expected.
2. along that script the man page ./doc/isnssetup.8

3. There also is isnsd.socket which could be installed along the .service
   Looking at the .service made me shiver as a lot of things might be discussed, but right now we
   only want to package 0.101 as-is.

« Back to merge proposal