… 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: Plack::App::FakeApache1
Plack::App::FakeApache1 is a Perl distro to aid in mod_perl1->PSGI migration.
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 in January 2014 (https://github.com/chiselwright/plack-app-fakeapache1)
- 0 pull requests outstanding, 2 closed, no PRs in 2015
- 2 open issues in GitHub; no RT queue
- latest release: 22nd of Jan, 2014 (https://metacpan.org/pod/Plack::App::FakeApache1)
- 1 reverse dependency via MetaCPAN
- Experimental kwalitee metrics failing on CPANTS (http://cpants.cpanauthors.org/release/CHISEL/Plack-App-FakeApache1-0.0.5)
- META.yml needs provides
- not all test-related modules are listed in
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.
README.mdto be helpful to people viewing GitHub project
- needs a proper
- 12 failing tests; 246 passing, 8 n/a
- copyright info needs updating
- doesn’t provide a
dzil authordeps --missing | cpanm
Dist::Zillaplugins were added
dzil listdeps --missing
- get a warning that
NoTabsTestshould be replaced with
- get a warning that
dzil listdeps --missing | cpanm
dzil testruns and the tests pass (without the release tests)
- warnings are thrown about
PodWeavernot finding an abstract in some files
- warnings from
PkgVersionabout no blank line for
$VERSIONafter package statement
- warnings are thrown about
.travis.yml-> added in PR#8
dzil test --release
- while adding a
.travis.yml, found that the tests won’t pass without the git user.name and user.email settings being in the config. This is the fault of one of the
Dist::Zillaplugins. Unfortunately, setting the
--automatedflag to the
dzil testcommand doesn’t help. It would be nice if the distribution were able to take this into account. Ended up setting up a Vagrant VM to investigate the issue. Inside the VM the problem with the git setup doesn’t appear. And
git config --listdoesn’t mention the
user.emailsettings. The error in Travis is:
- Vagrant VM environment notes that
Pod::Coverage::TrustPodneed to be installed as a prereq. Where to put this information in
- come to think of it, why are the pod-coverage tests running on Travis
--releaseoption isn’t passed to
dzil test??? It looks like the pod tests are being set as author tests on Travis, but as release tests when running on a local environment. It turned out this was because the test is prefixed with the word
authoron Travis, but with the word
releaseon my local box.
dzil test --verboseon Travis unfortunately doesn’t uncover exactly where the git issue is coming from. It looks like it’s coming from
Dist::Zillaitself, since it appears just after this message:
- after checking the
Dist::Zillaplugins via divide-and-conquer, it turns out that
Git::CommitBuildis the plugin which causes the issue.
- an upgrade of
Dist::Zillaon my local box made the author tests (for this distribution the POD tests) run per default via
dzil test. This means that the pod-coverage issue is now repeatable on the local Vagrant VM, the local bare metal box and the Travis setup. At least now we’re consistent! The reason for the inconsistency was a version difference between
Dist::Zillaon my local box and on the Vagrant and Travis machines. There had been a change in
Dist::Zillabehaviour which meant that
dzil testruns author tests by default, hence why on the more up to date systems the author tests were getting run.
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:
POD errors fixed in PR#10.
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
Need to fix 7 files with trailing whitespace. Fixed in PR#9.
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.
Found some strictures issues. Fixed one strictures issue. The two remaining issues aren’t really a problem (a warning about turning off a stricture and a warning about string eval).
Strictures issues fixed in PR#12.
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:
29.9% total coverage
Which is quite low…
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.
Everything seems to work and to point to the correct location.
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.
Fixed some typos found in the POD in PR#14.
Tests fail with non-English locale (GitHub issue #5) => added a fix for the issue in PR #15.
The module already uses Kwalitee tests as part of the
--release option to
dzil test, thus this doesn’t need to be followed much further.
Overview of the pull requests made
- add missing
ABSTRACTstatements to packages
- add Travis configuration
- purge trailing whitespace
- fix POD errors
- update the
- update the copyright date
- don’t distribute vim backup files
- ensure all test-related prereqs are installed (this should also ensure that the build prerequisites are specified as required by the CPANTS experimental metrics)
- ensure that a
META.jsonfile is created
- ensure that “provides” information is available in
- add missing strictures to test
- remove perl shebang line in test scripts
- fix typos in the POD
- fix GitHub issue #5 (Tests fail with non-English locale)
- implement part of a fix for GitHub issue #4 (error severity level issue)
Most PRs were merged on the first night (1st of Nov)! Many thanks to CHISEL for the quick reaction!