Helping to clean CPAN: December 2015
… 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.
- last commit 13th of June 2015 (https://github.com/mstratman/Barcode-DataMatrix)
- 1 pull requests outstanding, 2 closed, all PRs in 2015
- it looks like Jeremy Mates (thrig) had this dist in June for the PRC; the PR contains fix-ups that look PRC-esque
- 0 open issues in GitHub; 2 open RT tickets (https://rt.cpan.org/Public/Dist/Display.html?Name=Barcode-DataMatrix)
- latest release: 16th of Apr, 2015 (https://metacpan.org/pod/Barcode::DataMatrix)
- 3 reverse dependencies via MetaCPAN
- Core kwalitee metrics failing (from http://cpants.cpanauthors.org/dist/Barcode-DataMatrix):
- no mymeta files (MYMETA files shouldn’t be included)
- this issue was fixed in PR#2
- Extra kwalitee metrics failing:
- meta yml declares perl version (should declare perl version)
- use warnings (should use warnings pragma)
- Experimental kwalitee metrics failing:
- meta yml has provides (should mention modules dist provides)
- has separate license file (should have separate license file)
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
- the
auto_set_repository
issue is thatModule::Install::Repository
is required in order to build the distribution. Installing this module viacpanm
fixed the issue. One can’t put the dependency in theMakefile.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 theauthor_tests
directive can be recognised inMakefile.PL
- adding a .travis.yml showed that
- copyright info needs updating -> fixed in PR#4
- doesn’t provide a META.json (however, don’t know if this is important)
GFI
,GFL
andPOLY
are declared inBarcode::DataMatrix::Constants
and inBarcode::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:
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
:
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.
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
In Dist::Zilla
projects, one needs to install the
Dist::Zilla::App::Command::cover
plugin, after which the code coverage can
be checked via:
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.
- 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:
Now look for .bak
file and check differences between it and the output
.txt
file, the process looks roughly like this:
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
- update copyright year
- skip POD tests in normal test suite (they are in
xt/
dir, so should only be run as part ofRELEASE_TESTING
) - add Travis configuration
- fix broken URLs -> partially replaced by README conversion
- add missing strictures
- purge trailing whitespace
- set minimum Perl version
- replace
Any::Moose
withMoose
-> replaced by PR#13 - add explicit LICENSE file
- replace
Any::Moose
withMoo
/Type::Tiny
- fix incorrect code indentation
- convert README to markdown and restructure it
- add docs to all subs
- remove subroutine prototypes
- update outdated URLs (the URLs in embedded POD still needed to be fixed)
- skip/ignore
cover_db
- remove unused subroutines
- remove unused constants
- remove now unnecessary “no critic” statement
- remove commented-out code
- add missing
use warnings
pragmas - reformat
DataMatrix
code; now things are consistently formatted - add META.json
- add tests of Engine and CharDataFiller classes
- remove unused variable
$k
inEncodeBASE256
Conclusion
Most pull requests have been merged! And a new version of the distribution has been released. Yay!
Support
If you liked this post and want to see more like this, please buy me a coffee!