Bug#884697: logrotate package
thank you for taking some time and looking over the package.
> I've looked over the 3.14.0-1 package version and generally everything>
> looks very good to me. I'm appending my review notes about minor things
> below which might be useful for future improvements none the less.
> Please tell me if you want me to go ahead with further testing and
> uploading of the package, or if you already have someone else in mind
> for this task.
Yes, I'd like to go ahead.
> Please also mention if you've contacted and what their response have
> been for the people offering mentorship (like in
> https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=887151#17 ).
I messaged them
(https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=887151#45) but got
unfortunately no response yet.
> WATCH OUT / HEADS UP:
> - I'm not sure about the current state but this has bitten me in the
> past. The debhelper systemd integration in the past had no particular
> knowledge about timer units. That resulted in the service unit for the
> respective timer unit being both enabled and *started* (or restarted,
> depending on if the package is newly installed or upgraded) at package
> installation/configure time. You likely do not want to trigger a
> logrotate at package installation/upgrade time and delay the entire dpkg
> operation until it completes. (I imagine some people might have massive
> logs that might take a very long time.) Please verify if current
> dh-systemd has improved on this front or if you need to add overrides
> for logrotate.service to not be started/enabled. I suspect this might
> very well be fixed now to just not start or enable services which don't
> have any [Install] section (like logrotate.service, but adding a
> build-time check to verify upstream doesn't slip one in there might be a
> useful safety for the future?).
Thanks for the hint; I checked this and package installation/upgrade
does not start the service (only the timer).
I am not sure though how to write a build time check.
> - Repository url should have '.git' appended instead of last '/', right?
> - I think Bug-Database is more revelant for listing issues url instead
> of using Contact.
> - I'm not sure using github project url in Homepage field is
> appropriate. It's supposed to be an url relevant for end users AFAIK.
> eg. github pages url would be suitable (if available, which it doesn't
> seem to be for logrotate).
There is unfortunately no homepage as such.
> - how old is this? There are no version checks and I didn't look, but
> maybe it can be dropped now? The less manual written maintainerscript
> code the better.
Seems this was changed in 3.11. so I'd like to keep this for buster.
> - this seems pretty outdated info as well considering for buster. Maybe
> it should also be dropped?
> - neat, but even better would be line-wrapping configure override using
> a backslash to put each config option on a separate line for easier
> - given existance of tests reduces unstable->testing migration delay,
> this might just be a bit too trivial test to exist alone???
> (while at the same time an existing test might be better than no test
> at all....)
I do not know what the test environment offers, so I only wrote that
In my opinion the build time tests plus the test that logrotate is
running (even only printing the version and build information) is
sufficient for now.
But I welcome any test ideas.
> - why is this information relevant to put in the manpage? A more general
> solution would be to describe that apt-file can be used to search for
> which package contains something. Not suitable to document in
> specialized manpages like logrotate IMHO. Oh, I see this patch is not
> listed in series file so not applied. Well, might be another reason to
> drop it.