Paul Cochrane

Paul Cochrane

Multi-purpose software developer and geek

Helping to clean CPAN: February 2016

8 minute read

… 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.

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.

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
  • dependency Test::Mini fails to install
    • it turns out the issue is fixed in the Test::Mini repository; the module just needs to be released
    • adding the Test::Mini and Test::Mini::Unit dependencies via cpanm --notest allows the test suite to pass (on Perl 5.20.2)
  • perl Makefile.PL now runs ok
  • no .travis.yml
  • also doesn’t use coveralls
  • META.yml and META.json are in the distribution, but not in the repo
  • running for test_file in t/*.t; do perl -Ilib $test_file; done showed that one needs to run git submodule update --init before 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 Makefile needed to be recreated -> the problem was thus PEBKAC
    • prove -lr t runs as expected
  • does 001_spec_compatibility.t need 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 make test)
  • 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 make step required.
  • adding an explicit make step didn’t help; the spec tests still fail:
t/001_spec_compatibility.t ............................ Dubious, test returned 1 (wstat 256, 0x100)

Failed 2/83 subtests`
  • prove -lr t also fails
  • running files individually didn’t actually pass; the following passing tests masked the failures. The errors were:
not ok 67 - test - Nested (Truthy)

# Can't call method "can" without a package or object reference
  • and …
not ok 83 - test - Sections

# Can't call method "can" without a package or object reference

1..83
  • running tests with prove locally 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.

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 ./ -name '*.pm' -or -name '*.pod' | xargs podchecker |& grep ERROR # 0 errors
$ find ./ -name '*.pm' -or -name '*.pod' | 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)

No files need to be fixed.

Add files to .gitignore

Need to add MYMETA.*, MANIFEST, the distribution tarball, blib and cover_db to .gitignore.

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
  • bareword filehandle
  • two argument open
  • variable declared in conditional statement
$ perlcritic t
  • bareword filehandles
  • strictures errors
  • and many more…

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:

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.

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://'

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 (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.

One minor typo in main file: “overriden -> overridden”.

Kwalitee tests

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. Test::Kwalitee::Extra uses set of metrics closer to that used on CPANTS, so replace the kwalitee_ok 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 ExtUtils::MakeMaker or Build::Module distribution like so:

$ RELEASE_TESTING=1 prove t/99kwalitee.t

or, if the distribution uses Dist::Zilla, run

$ dzil test --author --release

Local kwalitee tests agree with CPANTS.

GitHub issues

  • 4 to look into

RT issues

  • no issues mentioned in the RT queue.

Overview of the pull requests made

Conclusion

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!

You may also enjoy

German Perl/Raku Workshop Frankfurt 2024

12 minute read

This week I was at the German Perl/Raku Workshop in Frankfurt. This is the main yearly meetup of the German-speaking Perl/Raku community. The event isn’t j...

Sailplane glide distance

7 minute read

As part of tutoring physics and maths to high school students, I sometimes write up deep-dive explanations of questions arising during lessons. The question...

Starting ssh-agent in Windows PowerShell

2 minute read

As someone who works mostly on Linux, there seem to be a multitude of issues for me to stumble over when having to use Windows. Using ssh in Windows PowerSh...