Paul Cochrane

Paul Cochrane

Multi-purpose software developer and geek

Helping to clean CPAN: August 2016

9 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: Mo

Mo: Perl Micro Objects

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, let’s 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).

  • Travis config could be updated to more Perl versions
  • there is no META.yml or META.json in the repository; looks like they’re automatically generated, since they’re in the dist
  • uses Dist::Zilla
  • needed to run dzil authordeps --missing | cpanm in order to run dzil test
  • dzil test showed that dzil authordeps --missing | cpanm was necessary on my box
  • dzil test gives an error with t/author-test-version.t, although this could be PEBKAC
t/author-test-version.t ....... 1/? Error evaling version line ' my $dummy =
q#  Hide from _packages_inside()
    #; package Module::Metadata::_version::p8;
    use version;
    sub {
      local $Attr::Trait::VERSION;
      $Attr::Trait::VERSION = '0.40';use Mouse::Role;around
_process_options=>sub{my$orig=shift;my$c=shift;my($n,$o)=@_;$o->{is}||='rw';$o->{lazy}||=1
if defined$o->{default}or
defined$o->{builder};$c->$orig(@_)};$INC{'Attr/Trait.pm'}=1};
      $Attr::Trait::VERSION
    };
  ' in
/home/cochrane/Projekte/OSSProjekte/mo-pm/.build/rmEaaxGDmu/blib/lib/Mo/Mouse.pm:
Unmatched right curly bracket at (eval 58) line 8, at end of line
syntax error at (eval 58) line 8, near "$Attr::Trait::VERSION
    }"

failed to build version sub for
/home/cochrane/Projekte/OSSProjekte/mo-pm/.build/rmEaaxGDmu/blib/lib/Mo/Mouse.pm
at
/home/cochrane/perl5/perlbrew/perls/perl-5.18.4/lib/site_perl/5.18.4/Test/Version.pm
line 87.
  • some of the Dist::Zilla plugins are deprecated and need to be replaced
!!! [ReportVersions::Tiny] is deprecated; recommended alternative:
[Test::ReportPrereqs] at
/home/cochrane/perl5/perlbrew/perls/perl-5.18.4/lib/site_perl/5.18.4/Dist/Zilla/Plugin/ReportVersions/Tiny.pm
line 12, <GEN0> line 33.
!!! [NoTabsTests] is deprecated and will be removed in a future release;
replace it with [Test::NoTabs]
 at
/home/cochrane/perl5/perlbrew/perls/perl-5.18.4/lib/site_perl/5.18.4/x86_64-linux/Class/MOP/Method/Wrapped.pm
line 42.
!!! [EOLTests] is deprecated and may be removed in a future release; replace
it with [Test::EOL] (note the different default filename)
 at
/home/cochrane/perl5/perlbrew/perls/perl-5.18.4/lib/site_perl/5.18.4/x86_64-linux/Class/MOP/Method/Wrapped.pm
line 42.
  • the Dist::Zilla plugin @Filter/PkgVersion shows many warnings
[@Filter/PkgVersion] no blank line for $VERSION after package Mo statement
in lib/Mo.pm line 1
[@Filter/PkgVersion] skipping lib/Mo/Golf.pm: assigns to $VERSION
[@Filter/PkgVersion] skipping lib/Mo/Inline.pm: assigns to $VERSION
[@Filter/PkgVersion] no blank line for $VERSION after package Mo::Moose
statement in lib/Mo/Moose.pm line 1
[@Filter/PkgVersion] no blank line for $VERSION after package Attr::Trait
statement in lib/Mo/Moose.pm line 4
[@Filter/PkgVersion] no blank line for $VERSION after package Mo::Mouse
statement in lib/Mo/Mouse.pm line 1
[@Filter/PkgVersion] no blank line for $VERSION after package Attr::Trait
statement in lib/Mo/Mouse.pm line 4
[@Filter/PkgVersion] no blank line for $VERSION after package Mo::build
statement in lib/Mo/build.pm line 1
[@Filter/PkgVersion] no blank line for $VERSION after package Mo::builder
statement in lib/Mo/builder.pm line 1
[@Filter/PkgVersion] no blank line for $VERSION after package Mo::chain
statement in lib/Mo/chain.pm line 1
[@Filter/PkgVersion] no blank line for $VERSION after package Mo::coerce
statement in lib/Mo/coerce.pm line 1
[@Filter/PkgVersion] no blank line for $VERSION after package Mo::default
statement in lib/Mo/default.pm line 1
[@Filter/PkgVersion] no blank line for $VERSION after package Mo::exporter
statement in lib/Mo/exporter.pm line 1
[@Filter/PkgVersion] no blank line for $VERSION after package Mo::import
statement in lib/Mo/import.pm line 1
[@Filter/PkgVersion] no blank line for $VERSION after package Mo::importer
statement in lib/Mo/importer.pm line 1
[@Filter/PkgVersion] no blank line for $VERSION after package Mo::is
statement in lib/Mo/is.pm line 1
[@Filter/PkgVersion] no blank line for $VERSION after package Mo::nonlazy
statement in lib/Mo/nonlazy.pm line 1
[@Filter/PkgVersion] no blank line for $VERSION after package Mo::option
statement in lib/Mo/option.pm line 1
[@Filter/PkgVersion] no blank line for $VERSION after package Mo::required
statement in lib/Mo/required.pm line 1
[@Filter/PkgVersion] no blank line for $VERSION after package Mo::xs
statement in lib/Mo/xs.pm line 1

The code gets “golfed” from

documented and fairly readable [code] … and [reduces] it to a single undecipherable line.

Hence, these warnings are simply an artefact of the golfing process.

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:

97.9% statement coverage; 92.9% total coverage.

Which is excellent coverage. Strangely enough, dzil cover runs to completion, whereas dzil test doesn’t (see error in author test version above).

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' -or -name '*.pod' | xargs podchecker |& grep ERROR # 0 errors
$ find ./lib -name '*.pm' -or -name '*.pod' | xargs podchecker |& grep WARN # 1 warning
    *** WARNING: line containing nothing but whitespace in paragraph at line 70 in file ./lib/Mo/Design.pod

This issue is fixed in PR#33.

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)

There’s already a Dist::Zilla plugin checking for this and the issue was resolved in PR#33.

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

These commands generate many warnings, however this is the kind of module where one knowingly doesn’t use strictures and does various kinds of magic, so perlcritic isn’t as helpful as it usually is.

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://'
git grep 'ftp://'
git grep 'git://'

All links look good.

This probably also sits in the nit-picking category for some people, however, copyright dates (theoretically) need to be kept up to date. The appropriate copyright year is usually the year of the last release. However, if a release looks imminent, then the current year is likely to be the right candidate. Some distributions put the author’s email address on the same line as the copyright date, hence this needs to be checked as well.

Here we do a case-insensitive grep over the source for the word “copyright”, the line of which we check for the existence of a year (i.e. 4 digits), look for the appropriate year and then clean up the grep results to get something we can pass directly to vim.

$ COPYRIGHT_YEAR=2016
$ git grep -i copyright
$ vim $(git grep -i copyright | grep -P '\d{4}' | grep -v $COPYRIGHT_YEAR | cut -d':' -f1 | uniq)
=> 5 files
$ vim $(git grep -i copyright | grep -P '\d{4}' | grep -v '@' | cut -d':' -f1 | uniq)

Didn’t bother to add a missing email address to the copyright statement since this didn’t seem to be the pattern in this distribution.

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.

POD spell checks ok.

Kwalitee tests

Although CPANTS is the main kwalitee reference, one can also run the kwalitee tests locally. One can use the t/kwalitee.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

Not sure if the kwalitee tests are useful for this module. A lot of magic happens and as a result many of the standard rules are broken so that this can happen. Hence the quality checks don’t bring as much value for a module of this kind.

GitHub issues

Unfortunately, there wasn’t any time this month to really dig into GitHub issues, hence none of the open issues were addressed as part of this month’s challenge.

RT issues

There are no issues in RT.

Overview of the pull requests made

Conclusion

Although the patches were of a very trivial nature, they managed to get the dist closer to passing its tests on all Perl versions again on Travis; the upstream dependency (INGY bundle) needs to be updated on CPAN so that the tests can pass again.

Many thanks to TINITA, INGY, and DOLMEN for their feedback and quick responses in order to get these patches merged!

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