Control: reopen -1
Control: tags -1 moreinfo

Hi Lucas,

thanks for fixing most of the issues raised in my review. I'll
address the remaining ones below.

> > * changelog: revision 1.0.21-1.1 is missing, please restore.

This is still missing. If an upload makes it into the archive and you
agree with it being done, you normally want to include it even though
it wasn't prepared by you. See the developers reference, 5.11.4 [2]:
"To acknowledge an NMU, include its changes and changelog entry in
your next maintainer upload".

> >    + why the (undocumented!) bump of the versioning of the
> >      conflicts/replaces?  
> Fixed.

As a rule of thumb, every packagin change should be documented in
d/changelog - including things that may seem trivial or obvious.

As for the version bump itself, the question of 'why?' remains. The
versioning on the conflicts/replaces gets bumped to "<< 1.0.21-2", but
there are no packaging changes between 1.0.21-1 and 1.0.21-2 that
require doing so. The changelog entry tries to come up with an
explanation (good!), but unfortunately it does not make much sense.

After all, the incompatibility is between the package designs from
before and after the renaming of the binary package from "lsm" to
"foolsm"; the versioning on the conflicts/replaces should reflect
that. And that also implies it should remain at "<< 1.0.21-1",
unless/until a future package revision introduces some other
incompatibility that demands a conflicts/replaces in its own right.

> > * init script:
> >    + exits nonzero if the executable isn't installed anymore,
> > [...]
> I really don't know if I understood this one, but maintscript
> handle the upgrade on the transition.

My comment was about the part of the script that tests for the
presence of the program and then stops with a non-zero exit status:
---
if [ ! -x $DAEMON ]; then
  [...]
  log_end_msg 1
fi
---
The script should instead set its exit status to zero here, because
this situation (init script installed but $DAEMON missing) is
expected to happen whenever a user uninstalls the package. See Debian
policy, section 9.3.2 [1]; in particular the paragraph that starts
with "These scripts should not fail obscurely when the configuration
files remain but the package has been removed".

> > * rules: doesn't respect 'nodoc' build profile. See [2] for
> > examples to make the documentation build conditional.
> 
> nodoc had been append to build-depends field but not used as
> expected,

That's because nodoc isn't the default build profile, so it wouldn't
be used unless explicitly ordered. The call to 'docbook-to-man' in
d/rules should be conditional on the build profile, like so:
---
override_dh_auto_build:
ifeq (,$(filter nodoc,$(DEB_BUILD_OPTIONS)))
        docbook-to-man debian/foolsm.sgml > debian/foolsm.1
endif
        dh_auto_build -- PREFIX=${PREFIX}
---
Otherwise, building with the nodoc build profile would not install
the build-dependency on docbook-to-man, but still try to run it.


Then, one freshly introduced issue:
* there's an obvious typo ("$ start") in the init script restart code.

And one that yours truly overlooked last time:
* please switch the upstream homepage in d/control to https as well.


[1]https://www.debian.org/doc/debian-policy/ch-opersys.html#writing-the-scripts
[2]https://www.debian.org/doc/manuals/developers-reference/pkgs.html#non-maintainer-uploads-nmus

Attachment: pgpOVyPU0py8J.pgp
Description: OpenPGP digital signature

Reply via email to