… 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: Template::Mustache
Template::Mustache is an implementation of the fabulous Mustache templating language for Perl 5.8 and later.
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 11th of Dec 2015 https://github.com/pvande/Template-Mustache
- 0 pull requests outstanding, 10 closed
- 4 open issues in GitHub
- latest release: 11th of Dec, 2015 (https://metacpan.org/release/Template-Mustache)
- 2 reverse dependencies via metacpan
- Core kwalitee metrics failing (as per CPANTS http://cpants.cpanauthors.org/release/YANICK/Template-Mustache-v0.5.3):
- has human readable license
- has license in source file
- has abstract in pod
- Extra kwalitee metrics failing:
- meta yml declares perl version
- has known license in source file
- Experimental kwalitee metrics failing:
- meta yml has provides (should mention modules dist provides)
- has separate license file (should have separate license file)
- CPAN testers shows all green: http://matrix.cpantesters.org/?dist=Template-Mustache+v0.5.3
There is some low-hanging fruit here for things to clean up. The repository was updated reasonably recently, which is a good sign of activity. Only two other distributions are relying on it, so it’s not used widely within the Perl ecosystem. The fact that all pull requests are closed is also a good sign of activity on the project.
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.
- EUMM project
- build and installation instructions helpful in README.md?
cpanm --installdeps .necessary for dependencies
Test::Minifails to install
- it turns out the issue is fixed in the
Test::Minirepository; the module just needs to be released
- adding the
cpanm --notestallows the test suite to pass (on Perl 5.20.2)
- it turns out the issue is fixed in the
perl Makefile.PLnow runs ok
- also doesn’t use coveralls
- META.yml and META.json are in the distribution, but not in the repo
for test_file in t/*.t; do perl -Ilib $test_file; doneshowed that one needs to run
git submodule update --initbefore the SpecCompatibility tests can run. After running this command, the tests break when running
make test, however not when running the tests individually… Odd… The problem was, that the
Makefileneeded to be recreated -> the problem was thus PEBKAC
prove -lr truns as expected
001_spec_compatibility.tneed to be executable?
- doesn’t use POD for docs -> should probably be be changed to POD
- while adding .travis.yml, found that all Perl versions below 5.18 fail the test suite
- locally, 5.16.3, 5.14.4 pass the test suite (when run with
- git submodule command added to travis script; didn’t help
- tried running test files individually (
perl Makefile.PL; make; for ...) etc. this worked (actually, no, see below). Maybe is an explicit
- adding an explicit
makestep didn’t help; the spec tests still fail:
prove -lr talso fails
- running files individually didn’t actually pass; the following passing tests masked the failures. The errors were:
- and …
- running tests with
provelocally with Perl 5.16.3 now shows up the failing tests. Also when the tests are run via
make test. It is important to have cleaned and remade the project before running the test suite after changing Perl version.
- according to
perlver, the minimum Perl version is 5.8.0
What do all these notes mean? Well, they mean that installation instructions in the README would be really helpful, that Travis tests are a good idea and that there are few things one could do to make the distribution helpful to newcomers and users.
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.
podchecker gives the following errors and warnings:
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
No files need to be fixed.
Add files to .gitignore
Need to add
MANIFEST, the distribution tarball,
Perl::Critic will show up many
potential issues for Perl code. By simply running
perlcritic on the
t directories, one can get a further handle on the code’s quality.
- bareword filehandle
- two argument open
- variable declared in conditional statement
- bareword filehandles
- strictures errors
- and many more…
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
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:
89.8% total coverage
…since not all conditional paths are taken, however all statements are covered by the test suite. The test coverage is thus very good.
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.
The link to the mustache man page needs to be updated.
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
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
.pod files as necessary.
One minor typo in main file: “overriden -> overridden”.
Although CPANTS is the main kwalitee
reference, one can also run the kwalitee tests locally. One can use the
t/99kwalitee.t test script from
http://peateasea.de/cpan-pull-request-challenge/ for this purpose.
However, the script only uses
Test::Kwalitee which doesn’t cover as many
metrics as CPANTS.
set of metrics closer to that used on CPANTS, so replace the
call with simply
use Test::Kwalitee::Extra. More information about the
many options to
Test::Kwalitee::Extra can be found on the MetaCPAN page.
Run the kwalitee tests in an
distribution like so:
or, if the distribution uses
Local kwalitee tests agree with CPANTS.
- 4 to look into
- no issues mentioned in the RT queue.
Overview of the pull requests made
- add installation instructions to README - https://github.com/pvande/Template-Mustache/pull/23
- convert docs to pod - https://github.com/pvande/Template-Mustache/pull/24
- ignore more automatically generated files - https://github.com/pvande/Template-Mustache/pull/25
- add a license file - https://github.com/pvande/Template-Mustache/pull/26
- replace bareword filehandle with variable in main code - https://github.com/pvande/Template-Mustache/pull/27
- specify the minimum Perl version - https://github.com/pvande/Template-Mustache/pull/28
- fix URL to mustache man page - https://github.com/pvande/Template-Mustache/pull/29
- add missing warnings and strict pragmas - https://github.com/pvande/Template-Mustache/pull/30
- replace bareword filehandles with variables in scripts and tests - https://github.com/pvande/Template-Mustache/pull/31
- ignore RequireFilenameMatchesPackage perlcritic warnings - https://github.com/pvande/Template-Mustache/pull/32
- fix spec test suite - https://github.com/pvande/Template-Mustache/pull/33
- add travis configuration - https://github.com/pvande/Template-Mustache/pull/34
Nothing merged as yet and unfortunately no reaction from the author, however it can sometimes take months before someone realises that pull requests have been submitted to their repository, so these changes will hopefully get some attention in the future.
Update (06/11/2016): YANIK found the pull requests and merged them all! Yay!