licensecheck: improvements

[Benjamin Drung]

Am Mittwoch, den 10.10.2012, 04:09 +1100 schrieb Dmitry Smirnov:
> Dear devscripts team,
> 
> Benjamin recently wrote in #688843: "PPS: Patches are always welcome. :)"
> so I'm accepting the challenge. :)
>
> Please find first set of patches for licensecheck attached.

Thanks.

> Patches introduce conservative improvements and test case which is important 
> to have before changing any regexes due to risk of regression.

That's my opinion, too. Thats why I like to have a big bunch of test
cases.

> I'd like to continue working on licensecheck so please share your thoughts, 
> wishes and implementation ideas.

* 0001-licensecheck-help-docs-update.patch:

"B<licensecheck> [options] filename [filename ...]" is the short form of
"B<licensecheck> [B<--no-conf>] [B<--verbose>] [B<--copyright>] [B<-l>|
B<--lines=>I<N>] [B<-i>|B<--ignore=>I<regex>] [B<-c>|<--check=>I<regex>]
[B<-m>|B<--machine>] [B<-r>|B<--recursive>] I<list of files and
directories to check>". Having both makes no sense.

The current output of --help looks better to me IMO. It's shorter and
more precise. I expect a long and verbose text in the man page instead.

Before touching the command line interface, more command line parameter
tests would be nice (see test/test_licensecheck).

* 0002-licensecheck-new-option-print-defaults.patch

Depends on the previous patch. I don't need --print-defaults, but other
may.

* 0003-licensecheck-removing-license-statement-from-version.patch

Adding "(at your opinion)" is okay. Maybe the lines count could be
reduced by one.

I dunno if the license should be stated if --version is called. What do
the other maintainers think? Printing the copyright without the license
is strange. Either print both or none. ls and cp for instance print the
license.

* 0004-licensecheck-move-comments-clean-up-code-to-function.patch

The clean_comments function does more than just cleaning the comments.
It removes tabs and newlines for instance. Therefore a better name for
the function should be used. The return statement is not correctly
indented.

* 0005-licensecheck-removing-function-prototypes-perl-best-.patch

Looks good, but it does not apply out of order.

* 0006-licensecheck-perl-best-practice.patch

Applied in jessie branch.

* 0007-properly-report-empty-license.patch

I like to have a test case for this before applying it.

* 0008-introducing-embedded-tests.patch

I like to see these test cases as shunit2 test added to
test/test_licensecheck instead of embedding them. Using the shunit2
framework has two advantages: It does not make the script slower (as
Perl is a interpreted language) and the I/O part is tested as well.

* 0009-removing-w-as-we-already-use-warnings.patch

Applied in jessie branch.

PS: Sorry for being a perfectionist.

> Also I applied for team membership using form on Alioth but got no reply yet.
> (my user name is onlyjob-guest and I'm a DM).

I just have a Junior Developer role and therefore get no membership
requests.

-- 
Benjamin Drung
Debian & Ubuntu Developer
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part
URL: <http://lists.alioth.debian.org/pipermail/devscripts-devel/attachments/20121009/8fd208c7/attachment.pgp>