licensecheck: improvements

[Benjamin Drung]

Am Freitag, den 19.10.2012, 13:08 +1100 schrieb Dmitry Smirnov:
> On Fri, 19 Oct 2012 09:59:10 Benjamin Drung wrote:
> > 
> > My usual approach is the other way around: Improve the patches
> > iteratively until I am happy and then apply them.
> > [...]
> > I agree that reduce duplication is a good idea.
> > 
> 
> Here is another patch (attached) and latest licensecheck.pl from Jessie with 
> attached patch applied.
> 
> This time it is much nicer.
> 
> I moved version() to VERSION section of embedded POD, blended help() with POD 
> and introduced --print-defaults once again as I believe it doesn't belong to 
> help().
> 
> help() is re-implemented using pod2usage from Pod::Usage -- note that 
> 'autouse' load it dynamically only when we need to print something from 
> embedded documentation.
>
> Also I alphabetised list of options.

Make sense (unless there were a reason to not alphabetize it).

> At the moment --help prints longer list than what it used to do but I don't 
> see how this can be implemented without duplication. We could create another 
> POD section "HELP" but it will merely contain OPTIONS squashed into more 
> compact form. Probably it's not worth it and I hope you might agree that now 
> the output is more traditional and maintainable. I remember you mentioned that 
> you liked more compact --help output so I'm wondering how would you prefer to 
> re-implement it using embedded POD.

I come to the conclusion that --help and a man page serve different
purposes: The --help is a quick and compact help reference. Each command
line parameter is documented in one line (most cases). The man page is a
full documentation. It gives a description, use cases, and so on. A good
example is the gcc: Compare the two pages of --help with the 12,000
lines on text in the man page.

Can these different goals reached by only one text? I start do doubt
that. In most other scripts written in Python, we use optparse to parse
and generate the --help output, but maintain a man page separately. The
extra man page is often more verbose. The downside of this approach is
text (not code) duplication.

What do the other maintainer think in this regard? If other maintainers
think that the --help and the man page content should be merged, I will
not block the patch.

> > Other software has as many license statements, too: in header, in man
> > page, in --version.
> > [..]
> > Can you prove that it is common sense that --version does not show a
> > license statement? I tested cp and ls and both state a license, too.
> > 
> 
> No worries, although IMHO it is unnecessary to print license on --version I 
> don't mind doing it as long as we don't duplicate the code for that reason.
> 
> Please have a look how it is implemented in the attached files, I hope you'll 
> like it much better. --version still prints license and copyright statements 
> but it take 'em from embedded POD sections. Much more elegant IMO.

I prefer the style of other GNU tools. The first line contains the
version (easier to parse):

licensecheck (devscripts) ###version###

The following lines can then contain the copyright and license information.

> > I am not the author of licensecheck and Perl is not my favorite
> > programming language. So I may be more conservative applying patches.
> > That's why I may let you iterate over a patch until I am confident in
> > applying it.
> > 
> 
> No worries, Perl is my favourite language. :)
> I'm sure we just need a bit more time to understand each other better.

Feel free to join #devscripts on OFTC to talk to us on IRC.

> > I prefer the current test suite over the test in the licensecheck file
> > itself.
> > 
> 
> I had a look at "shunit2" and now I convinced that it's not good enough.
> It's not right to use shell test suite for Perl. Test::More is more flexible, 
> powerful and common for Perl. If license detection will mature we might move 
> it to standalone Perl module in which case native testing will be preferable.
> 
> Lately I implemented embedded tests with 
> 
> if($OPT{'tests'}){
>     eval q{
>         use SelfLoader;
>         tests();
>     };
>     exit;
> }
> 
> which even use SelfLoader only when needed so if you concerned for performance 
> it won't be affected *at all*.
> 
> I have many reasons to stick to embedded tests for now. For example because I 
> introduce tests first, some of them will be failing by definition until viable 
> fix will be implemented. I'm going to introduce new test with the new  
> reported bugs so some tests will be failing until we agree on viable solution 
> for the bug fix. This way tests will be also a road map for ongoing work.

This approach is unrelated to the used test framework.

> I don't want to interfere with shunit2 for those tests.
> 
> Also it is much easier and quicker to invoke "./licensecheck.pl --tests" 
> rather than require source package to be able to run tests.

Invoking "test/test_licensecheck" is as easy and quick as invoking
"./licensecheck.pl --tests". You can also run licensecheck manually on a
license file: "scripts/licensecheck.pl test/licensecheck/gpl-3.sh"

> I would like to hear a better argument for "shunit2" rather than that you 
> prefer it without explaining why.

shunit2 is not bound to a specific language. I could rewrite
licensecheck in another language and still use the test cases. shunit2
tests the user interface (= command line) instead of only internal Perl
functions. The test are easily manually reproducible by just running the
command from the shunit2 test manually.

What's the opinion of the other maintainers?

> I will insist on using Test::More. If I fail to convince you I will have to 
> focus my attention on implementing license detection and testing in standalone 
> module which would mean that I won't be working on licensecheck until new 
> module will mature enough to substitute most of licensecheck's code.
> 
> 
> > My comments on your patches are not meant to block you. It's to
> > improve/change the patches or further discuss the change with the other
> > maintainer and then apply your patch or later revisions of your patches.
> 
> Thank you. I'm sure our work will be productive.
> Also many thanks to you for applying some of my later patches. :)

You're welcome. :)

-- 
Benjamin Drung
Debian & Ubuntu Developer