[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#675532: RFS: bilibop/0.1 (ITP #675467)



Hi,
and thanks for this review

On 19/07/2013 20:25, intrigeri wrote:

> First, was the target distribution change in debian/changelog
> intentional? (0.4.12 has experimental, 0.4.13 has unstable.)

Yes it is; the target distribution was set to experimental for the
wheezy's freeze duration... after what it was forgotten. Do you think
I should revert to experimental ?

> Second, it looks like important changes and refactoring are flowing in
> rather quickly, so I'd like to check that you are confident with the
> current state of bilibop, and believe it is stable enough to be part
> of a Debian release. Do you confirm this?

The most important changes are about debconf support, which has been
added to bilibop-rules. I consider this as an improvement. Maintainer
scripts are idempotent. I have tested bilibop-rules 0.4.13 installation
in several ways: fresh install (with or without preseeding it), upgrade
from 0.4.11, remove, reinstall, purge: all seems to work as expected.

Other things are mainly addition of comments in the shell library,
update of the documentation, and some changes in the code to take into
account some very specific usecases.

So yes, I am confident with the current state of bilibop (but yes, this
was not the case with 0.4.12 - less than one day of lifetime!). Also
note that bilibop is still in active development, and some enhancements
are planned for the next months or even next years; of course, important
attention is given to not break existing things.

> Also, please keep in mind that once bilibop is uploaded to Debian, the
> responsibility of backward compatibility will be yours, as the
> maintainer. This being said, while I certainly wouldn't mind a bit
> more abstraction / factorization at some places, the code looks solid
> enough :)
> 
> Some nitpicking follows, that should be fixed before the initial
> upload IMHO:
> 
>> +        myshell="$(awk -F: "\$1 ~ /$(whoami)/ {print \$NF}" /etc/passwd)"
> 
> Maybe use `getent' instead?

Fixed.

> Also Lintian says:
> 
>   I: bilibop-common: spelling-error-in-manpage
>   usr/share/man/man1/drivemap.1.gz informations information
> 
> ... and a few others, so you probably want to run it in verbose /
> pedantic mode and take the results into account.

Mh... I already used '--info --verbose --pedantic' in my script. I
thought it was enough, as mentors.debian.net/package/bilibop says
'Package is lintian clean'.

The tags you mention are catched by '--display-info'. Fixed.

So, here is the new version:
http://mentors.debian.net/debian/pool/main/b/bilibop/bilibop_0.4.14.dsc

Cheers
quidame

Attachment: signature.asc
Description: OpenPGP digital signature


Reply to: