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.
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.
>
> 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 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.
>
> 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.
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.
I would like to hear a better argument for "shunit2" rather than that you
prefer it without explaining why.
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. :)
Cheers,
Dmitry.
From 788e22451989ee9e4a6cffa45b3f4b5bc5085c58 Mon Sep 17 00:00:00 2001 From: Dmitry Smirnov <[email protected]> Date: Tue, 9 Oct 2012 02:22:39 +1100 Subject: [PATCH] licensecheck: help/docs update: * updating and alphabetising list of options in POD * merging POD documentation and help() to reduce duplication. * merginv POD documentation and version() to reduce duplication. * adding new option --print-defaults, de-coupled from help() * print "Usage:" information using pod2usage. --- scripts/licensecheck.pl | 144 +++++++++++++++++++++-------------------------- 1 file changed, 65 insertions(+), 79 deletions(-) diff --git a/scripts/licensecheck.pl b/scripts/licensecheck.pl index dabb734..083b65a 100755 --- a/scripts/licensecheck.pl +++ b/scripts/licensecheck.pl @@ -24,12 +24,9 @@ licensecheck - simple license checker for source files =head1 SYNOPSIS -B<licensecheck> B<--help>|B<--version> +B<licensecheck> B<--help>|B<--version>|B<--print-defaults> -B<licensecheck> [B<--no-conf>] [B<--verbose>] [B<--copyright>] -[B<-l>|B<--lines=>I<N>] [B<-i>|B<--ignore=>I<regex>] [B<-c>|B<--check=>I<regex>] -[B<-m>|B<--machine>] [B<-r>|B<--recursive>] -I<list of files and directories to check> +B<licensecheck> [options] filename [filename ...] =head1 DESCRIPTION @@ -44,39 +41,28 @@ the files contained within to the list of files to process. =over 4 -=item B<--verbose>, B<--no-verbose> - -Specify whether to output the text being processed from each file before -the corresponding license information. - -Default is to be quiet. - -=item B<-l=>I<N>, B<--lines=>I<N> - -Specify the number of lines of each file's header which should be parsed -for license information. (Default is 60). +=item B<-c=>I<regex>, B<--check=>I<regex> -=item B<-i=>I<regex>, B<--ignore=>I<regex> +Specify a pattern indicating which files should be checked. +The default includes common source files. -When processing the list of files and directories, the regular -expression specified by this option will be used to indicate those which -should not be considered (e.g. backup files, VCS metadata). +=item B<--copyright> -=item B<-r>, B<--recursive> +Also display copyright text found within the file -Specify that the contents of directories should be added -recursively. +=item B<-h>, B<--help> -=item B<-c=>I<regex>, B<--check=>I<regex> +Display this message -Specify a pattern against which filenames will be matched in order to -decide which files to check the license of. +=item B<-i=>I<regex>, B<--ignore=>I<regex> -The default includes common source files. +Specify which files/directories matching the regular expression should +be ignored when checking files (e.g. backup files, VCS metadata). -=item B<--copyright> +=item B<-l=>I<N>, B<--lines=>I<N> -Also display copyright text found within the file +Specify how many lines of each file header should be parsed for license +information. (Default is 60). =item B<-m>, B<--machine> @@ -87,8 +73,26 @@ Note that using the B<--verbose> option will kill the readability. =item B<--no-conf>, B<--noconf> -Do not read any configuration files. This can only be used as the first -option given on the command-line. +Do not read any configuration files. +This must be the first command line option. + +=item B<--print-defaults> + +Print default settings and configuration overrides. + +=item B<-r>, B<--recursive> + +Scan the contents of directories recursively. + +=item B<--verbose>, B<--no-verbose> + +Print processed text from each file before its license information. + +Default is to be quiet. + +=item B<-v>, B<--version> + +Display version and copyright info. =back @@ -118,21 +122,26 @@ option. =head1 LICENSE -This code is copyright by Adam D. Barratt <I<[email protected]>>, -all rights reserved; based on a script of the same name from the KDE -SDK, which is copyright by <I<[email protected]>>. +Copyright (C) 2007,2008 Adam D. Barratt <I<[email protected]>>; +based on a script of the same name from the KDE SDK by <I<[email protected]>>. This program comes with ABSOLUTELY NO WARRANTY. You are free to redistribute this code under the terms of the GNU -General Public License, version 2 or later. +General Public License, version 2, or (at your option) any +later version. =head1 AUTHOR Adam D. Barratt <[email protected]> +=head1 VERSION + +B<licensecheck> version ###VERSION### from the Debian devscripts package. + =cut use strict; use warnings; +use autouse 'Pod::Usage'=>qw(pod2usage); use Getopt::Long qw(:config gnu_getopt); use File::Basename; @@ -228,10 +237,11 @@ GetOptions(\%OPT, "lines|l=i", "machine|m", "noconf|no-conf", + "print-defaults", "recursive|r", "verbose!", "version|v", -) or die "Usage: $progname [options] filelist\nRun $progname --help for more details\n"; +) or pod2usage(-verbose=>0,-exitval=>1); $OPT{'lines'} = $def_lines if $OPT{'lines'} !~ /^[1-9][0-9]*$/; $OPT{'ignore_regex'} = $default_ignore_regex if ! length $OPT{'ignore_regex'}; @@ -240,10 +250,26 @@ $OPT{'check_regex'} = $default_check_regex if ! length $OPT{'check_regex'}; if ($OPT{'noconf'}) { fatal("--no-conf is only acceptable as the first command-line option!"); } -if ($OPT{'help'}) { help(); exit 0; } -if ($OPT{'version'}) { version(); exit 0; } +if ($OPT{'version'}) { pod2usage(verbose=>99, sections=>[qw(LICENSE VERSION)], -exitval=>0); } +if ($OPT{'help'}) { pod2usage(verbose=>1,-exitval=>0); } +if ($OPT{'print-defaults'}){ + print <<"EOF"; +DEFAULTS + +--lines=$def_lines -die "Usage: $progname [options] filelist\nRun $progname --help for more details\n" unless @ARGV; +--check='$default_check_regex' + +--ignore='$default_ignore_regex' + +Default settings modified by devscripts configuration files: +$modified_conf_msg + +EOF +; + exit 0; +} +pod2usage(-verbose=>0,-exitval=>1) unless @ARGV; $OPT{'lines'} = $def_lines if not defined $OPT{'lines'}; @@ -378,47 +404,7 @@ sub clean_comments { return $_; } -sub help { - print <<"EOF"; -Usage: $progname [options] filename [filename ...] -Valid options are: - --help, -h Display this message - --version, -v Display version and copyright info - --no-conf, --noconf Don't read devscripts config files; must be - the first option given - --verbose Display the header of each file before its - license information - --lines, -l Specify how many lines of the file header - should be parsed for license information - (Default: $def_lines) - --check, -c Specify a pattern indicating which files should - be checked - (Default: '$default_check_regex') - --machine, -m Display in a machine readable way (good for awk) - --recursive, -r Add the contents of directories recursively - --copyright Also display the file's copyright - --ignore, -i Specify that files / directories matching the - regular expression should be ignored when - checking files - (Default: '$default_ignore_regex') - -Default settings modified by devscripts configuration files: -$modified_conf_msg -EOF -} -sub version { - print <<"EOF"; -This is $progname, from the Debian devscripts package, version ###VERSION### -Copyright (C) 2007, 2008 by Adam D. Barratt <adam\@adam-barratt.org.uk>; based -on a script of the same name from the KDE SDK by <dfaure\@kde.org>. - -This program comes with ABSOLUTELY NO WARRANTY. -You are free to redistribute this code under the terms of the -GNU General Public License, version 2, or (at your option) any -later version. -EOF -} sub parselicense { my ($licensetext) = @_; -- 1.7.10.4
licensecheck.pl
Description: Perl program
_______________________________________________ devscripts-devel mailing list [email protected] http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/devscripts-devel
