E. Choroba

CPAN Pull Request Challenge selected App::GitHooks for me in February. As usually, I started by browsing the issues, and my attention was drawn to Tim Bunce’s request to add a way to skip given hooks.

The run Method

The ticket suggested to use an environment variable to list the hooks to be skipped. The problem for me was to find the exact place where the skipping should happen. There were two run methods, one in App::GitHooks, and another in App::GitHooks::Hook. Both of them retrieved the name of the hook and ran the corresponding plugin.

I asked the author and maintainer for advice. The reply was almost instant: the latter module gets subclassed by particular hooks while calling SUPER::run isn’t required, so skipping the hook wouldn’t make much sense there.

In the former method, there were two places where early returning made sense: either as soon as the name of the hook was known, or just before running the run method of the given hook. The latter eventuality would still run lots of code—argument validation, loading of the hook class and instantiating it, and setting terminal encoding; all that for no use. The former way seemed more correct, but it involved returning from a try block. I wasn’t sure it was desirable, so I checked with the author again and he confirmed my solution as viable.

Feature Implementation Leads to Feature Request

The environment variable which drove the skipping was called $GITHOOKS_SKIP and its value was a comma separated list of hooks to skip. The pull request was almost ready when Tim Bunce intervened with a “user story” (as Scrum people would call it): he originally needed a way to disable all the hooks, not just particular ones. Moreover, he needed to skip as much as possible, because there were lots of commits and even loading the module thousand times was too slow.

Therefore, I introduced another variable, $GITHOOKS_DISABLE, which works in the same way as the previous one, but skips all the hooks when set to a true value. You still do some work for each commit and hook, but each plugin returns once its name is known even before its class was loaded, and you get the opportunity to inform the user that a hook was skipped.

When testing the new feature, I wanted to make sure a warning was emitted when a hook was skipped. I used a trick similar to the one described in my previous post: the check was done in a $SIG{__WARN__} callback.

It’s Your Turn

What remains is to add the notice about the skipped hook into the commit message. Maybe it will be you who draws the module for the next month?