Paul Cochrane bio photo

Paul Cochrane

Twitter LinkedIn Github

… where I get out my virtual broom and sweep up cruft I can find in my assigned distribution for this month’s edition of the CPAN Pull Request Challenge.

This month’s module: Kavorka

Kavorka provides function signatures with the lure of the animal.

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

  • looks like a Dist::Zilla project
  • ran dzil authordeps --missing | cpanm
  • dzil test says: “you can’t test without any TestRunner plugins”
  • dist.ini only contains comments … confusing. How to build the dist?
  • .travis.yml gives hints as to how to build distribution
  • asked author for help concerning installation, no reply as yet
  • asked on the #pr-challenge IRC channel for help. leont mentioned that Dist::Inkt is used for this project and that it’s the Kavorka author’s own build tool
  • installed Dist::Inkt (cpanm Dist::Inkt)
  • also needed to install Dist::Inkt::Profile::TOBYINK (via cpanm)
  • it seems one then needs to run distinkt-dist in order to build the project and run the test suite
  • Parse::Keyword is required as a dependency in order that the tests run
  • also needs namespace::sweep, Data::Alias and Return::Type
  • it seems that Moops is also needed:
ERROR: FAILED--Further testing stopped: Test requires module 'Moops' but it's not found

… which has Kavorka as a dependency. Huh? Moops is required for testing Kavorka, but Kavorka is required for Moops.

  • tests now pass (there’s a warning about needing a version bump since a git tag for version 0.036 is missing)
ERROR: try bumping the version before release at ...Dist/Inkt/Role/Test/BumpedVersion.pm line 35.
  • tried removing installed Kavorka to see if things still work
  • the tests still pass
  • … but the dist still doesn’t build. Seems to be due to the version bump which hasn’t been done
  • find all versions still at 0.035:
vim $(git grep VERSION | grep 0.035 | cut -d':' -f1 | sort | uniq)
27 files to edit
  • some instances turn up in sub-packages of .pm files
  • looks like meta/changes.pret also needs to be updated to the relevant version; used info from metacpan to try to simulate correct content
  • using a version equal to that of current version is also not the right thing to do:
ERROR: try bumping the version before release at .../Dist/Inkt/Role/Test/BumpedVersion.pm line 35.
  • interestingly enough, prove -lr t works
  • to solve the version problem I needed to bump $VERSION in all files to 0.037; then I needed to add a new entry to meta/changes.pret for this version; then the pre-build check for version numbers passed
  • unfortunately Text::sprintfn is needed in order to check that “Checking DOAP changeset metadata is current”
  • now distinkt-dist gets most of the way. It builds the dist, but has to bail out since there’s no gpg secret key available.
gpg: no default secret key: secret key not available
gpg: [stdin]: clearsign failed: secret key not available
Cannot find SIGNATURE.tmp, signing aborted.
ERROR: signature failed!!!
Bailing out at .../Dist/Inkt/Role/SignDistribution.pm line 45.
  • if PRs are accepted, will need to update the copyright year

What this analysis tells us is that there is a need for developer documentation so that one can get the project set up quickly. Once the project is set up and one can run things like the test suite, one can then focus on solving more involved problems. To try and help with this, a README with build instructions was added in PR#17.

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' | xargs podchecker |& grep ERROR # 0 errors
find ./lib -name '*.pm' | xargs podchecker |& grep WARN # 0 warnings

… ok, so the POD looks good.

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)
12 files to edit

Fixed in PR#16.

Overview of the pull requests made

Conclusion

Since the last commit was a year ago, and issues and pull requests are of a similar age, it seems that the author unfortunatley doesn’t have much time to spend on the project (very understandable considering that this is a complex piece of software provided as open source). Thus it might be a while before the pull requests submitted here get replied to or applied.