Re: RFS: lilo (updated package) (new Maintainer)
Hello Niels,
Niels Thykier <niels@thykier.net> wrote on 2010-12-12 16:05:
> I had a quick look at your package, though there are things I may have
> missed.
Thanks for your review.
> Too much copy paste from the RFS template :) The binary package has 11
> warnings (it also triggers a number of less severe tags e.g. I). Anyhow,
> considering you are targeting this for Squeeze, lintian-cleanless in
> itself is not a priority.
I know that problem. The most problems come from the special script
'liloconfig' which uses with debconf (but it should not do). It is a
legacy.
> Your priority being what it is; you should aim for fixing critical or
> highly annoying bugs with the minimal number of changes. If the diff
> gets to large, the release team may choose to reject it because they
> cannot keep track of the changes.
Thanks for this hint.
> Have you talked with the release team about the bugs you are fixing? I
> can understand the feeling of wanting to fix as much as possible, but
> the release team may feel the bugs are not important enough for the
> changes required.
I wrote to RT, there are some bugs which should be fixed for the
next release (Squeeze). I hope they see the special situation of LILO
maintaining. With the next major upgrade to 23.1 much more bugs can be
fixed (for Squeeze +1).
> > * Set source format 1.0. Add README.source file.
>
> I recommend undoing at least the README.source file; first off, it does
> not describe what it is supposed too and secondly even if you correct
> it, it has no functional change. The format part is only 1 line, so it
> is (probably) not going to disturb the reviewer too much.
I have done it because of a lintian warning (because of format 1.0). But
yes you are right. And I could use lintian-overrides. Should I do it for
this one warning? I think this is cosmetic, isn't it?
> I had a look and:
>
> - --- lilo-22.8/debian/lilo.postinst
> +++ lilo-22.8-9/debian/lilo.postinst
> [...]
> + # to be compatible to older debian package
> + if [ ! -e /boot/sarge.bmp ]; then
> + ln -sf debian.bmp /boot/sarge.bmp
> fi
> [...]
>
> That "debian.bmp", should that have been a "/boot/debian.bmp"?
As Jakub already have said this is o.k. It seems stupid to take account
of a file (or link) with the old "sarge" name, but until now this is the
main menu bitmap of lilo and I think enough user have activated the use
of this bitmap.
> In this snippet:
> - --- lilo-22.8/debian/rules
> +++ lilo-22.8-9/debian/rules
> @@ -23,6 +23,7 @@
> dh_testdir
> make spotless
> -rm -f build debian/files debian/lilo.substvars [...]
> + -rm -f debian/*.debhelper.log debian/lilo-doc.substvars
> -rm -f doc/*.dvi doc/*.ps doc/*.aux doc/*.toc doc/*.log
> -rm -rf debian/lilo debian/lilo-doc debian/*.debhelper [...]
> [...]
>
> Most of these lines (incl. the one added) appear to be re-implementing
> dh_clean. Replacing those lines with dh_clean /might/ be better (because
> the release team knows what dh_clean does and do not need to verify the
> handmade version)
That is also a legacy. But you are right: the best is to use dh_clean.
I will change this in the next update.
> There are also a lot of po file updates, which are not translation
> changes. I presume they have been made by some tool (e.g.
> debconf-updatepo from the clean target). As far as I can tell it has
> just added a Language header and in some cases modified the translation
Yes it it made because of debconf-updatepo, which runs in every debuild.
(see debian/rules). It is needless? Then I remove it in the next update.
Just in this minute I see my lilo package were sponsored by Julien BLACHE
<jblache@debian.org>. Should I nevertheless do an update of the package to
give the release team the chance to let it into Squeeze?
Have a nice day,
Joachim (Germany)
Reply to: