Paul Cochrane bio photo

Paul Cochrane

Twitter LinkedIn Github

… where I get out my virtual broom and sweep up cruft in my assigned distribution for this month’s edition of the CPAN Pull Request Challenge.

This month’s module: Barcode::DataMatrix

Barcode::DataMatrix generates data for Data Matrix barcodes.

First impressions

In this section I try to get a feeling for the state of the module, how up to date it is, how often people are contributing to it, how many other distributions are depending on it, how many bugs/issues it currently has, what the CPANTS kwalitee is, etc.

Initial inspection of the source code

After forking the repo and cloning a local copy, I have a look at the project to see what build system it uses, if the test suite works and the tests pass, if it could do with a Travis-CI config file (or if present, if it can be updated). The initial inspection often gives inspiration for the first batch of pull requests.

  • Module::Install project
  • README in plain text: convert to Markdown? -> changed in PR#15
$ perl Makefile.PL

Bareword "auto_set_repository" not allowed while "strict subs" in use at
Makefile.PL line 14.
Execution of Makefile.PL aborted due to compilation errors.
  • the auto_set_repository issue is that Module::Install::Repository is required in order to build the distribution. Installing this module via cpanm fixed the issue. One can’t put the dependency in the Makefile.PL since this is where the issue is happening. It would be a good idea to mention this in the README. This change implemented in PR#15
  • running make test gives 3 TODO tests which are passing (t/boilerplate.t) and 4 POD coverage tests which are failing
  • no .travis.yml
    • adding a .travis.yml showed that Module::Install::AuthorTests was also necessary in order that the author_tests directive can be recognised in Makefile.PL
  • copyright info needs updating -> fixed in PR#4
$ RELEASE_TESTING=1 make test

Failed test 'can't open /MANIFEST'
... among the pod test failures
  • doesn’t provide a META.json (however, don’t know if this is important)
  • GFI, GFL and POLY are declared in Barcode::DataMatrix::Constants and in Barcode::DataMatrix::Reed; this duplication needs to be removed
  • some routines aren’t used. E.g. isCDigit, decary
  • EncodeBASE256() shouldn’t even compile! There exists a line: my $k = which doesn’t end with a semicolon and the value for $k isn’t set to anything. One wonders why the compiler doesn’t complain. Fixed in PR#28

POD checks

The utility podchecker searches through Perl source code for POD which might not conform to the POD standard, and thus not necessarily be parseable by all POD parsers. Fixing any issues found by podchecker has the positive effect of also removing any warnings noted in the project’s documentation displayed on MetaCPAN.

Running podchecker gives the following errors and warnings:

$ find ./lib -name '*.pm' | xargs podchecker |& grep ERROR # 0 errors
$ find ./lib -name '*.pm' | xargs podchecker |& grep WARN # 0 warnings

Nit-picking

Check for trailing whitespace

Some projects consider this a must, and will disallow commits to be submitted which contain trailing whitespace (the Linux kernel is an example project where trailing whitespace isn’t permitted). Other projects see whitespace cleanup as simply nit-picking. Either way one sees it personally, this could be a useful pull request to a project, so it’s worthwhile fixing and submitting; the worst that can happen is that the pull request is closed unmerged.

To look for files with trailing whitespace, run git grep ' $'. It can be helpful to load the files found directly into vim:

$ vim $(git grep ' $' | cut -d':' -f1 | sort | uniq)

In this month’s module only two files were found with trailing whitespace; these were fixed in PR#9.

Add files to .gitignore

Need to add MYMETA.json and cover_db to .gitignore. cover_db ignored in PR#19; MYMETA.* ignored in PR#3.

perlcritic

Perl::Critic will show up many potential issues for Perl code. By simply running perlcritic on the lib and t directories, one can get a further handle on the code’s quality.

$ perlcritic lib
$ perlcritic t
$ perlcritic xt

Most errors are strictures errors. Subroutine prototypes are used and the expression form of eval is used in the tests. The biggest problem is probably just the strictures issue, which was fixed in PR#8.

Code Coverage

Looking at the code coverage can give an indication of code quality. If the project is well covered, this means most changes made in pull requests can be made with some confidence that any problems will be caught by the test suite. If the code coverage is low, then this is something that one could address as a pull request (or set of pull requests).

In EUMM and Build::Module projects, one simply needs to install Devel::Cover and run

$ cover -test

In Dist::Zilla projects, one needs to install the Dist::Zilla::App::Command::cover plugin, after which the code coverage can be checked via:

$ dzil cover

In this distribution, the coverage is:

65.3% total coverage

which probably could be better. Some new tests were added in PR#27 (as yet unmerged).

Stale URLs

Links to websites can go out of date, so it’s a good idea to see if they need updating or removing. A quick grep finds all the links. After which, we just need to see which links need fixing, if any.

$ git grep 'http://\|https://\|ftp://\|git://'
  • http://code.google.com/p/perl-ex/ is obsolete, should point to https://github.com/Mons/perl-ex/tree/master/GD-Barcode-DataMatrix
  • http://grandzebu.net/index.php?page=/informatique/codbar-en/datamatrix.htm should point to http://grandzebu.net/informatique/codbar-en/datamatrix.htm

These issues were fixed in PR#7.

Spell check POD

Good documentation can be a wonder to read. Not everyone’s docs are awesome, however we can keep the error rate to a minimum. A quick spell check will pick up most typos that don’t need to be there and fixing them can help improve the quality of a project.

In general, we want to find all files containing POD and run a spell checker (e.g. aspell) over all files, fixing typos we come across as we go. Not all projects require this much effort, however here’s a general-ish way to look for and check all POD in a project:

$ files_with_pod=$(find ./lib -name '*.pm' -o -name '*.pod' \
                 | xargs podchecker 2>&1 \
                 | grep 'pod syntax' | cut -d' ' -f1)
$ for filename in $files_with_pod
do
    pod2text $filename > $filename.txt;
    aspell -c $filename.txt;
done

Now look for .bak file and check differences between it and the output .txt file, the process looks roughly like this:

$ find ./ -name '*.bak'
$ diff -u lib/Path/To/Module.pm.pm.txt.bak lib/Path/To/Module.pm.txt

Then update the appropriate .pm and/or .pod files as necessary.

No issues found in this dist.

GitHub issues

  • none to address

RT issues

Overview of the pull requests made

Conclusion

Most pull requests have been merged! And a new version of the distribution has been released. Yay!