7/13/2016

How Things Get Done

The biggest obstacle to seizing an opportunity can be choosing exactly what to seize. Trips, projects, gatherings, clubs–the possibilities are endless. Time is not.

Luckily for us, exhaustive research and development have produced a foolproof recipe for getting the important things done. It reads:

  1. list everything you could be doing
  2. circle the most important thing
  3. do it

(4) is implicit: “Repeat”.



7/4/2016

Introduction to git hooks

Just beneath the surface of git’s version control system lies a rich framework for workflow development. At its heart are a sequence of events that are fired as changes are prepared, committed, and pushed. As each event is dispatched, git checks for–and executes–a corresponding hook. By implementing these in a development environment, we can automate tasks like:

  • verifying new changes against existing style guidelines
  • verifying and updating dependencies
  • enforcing history structure and hygiene

Let’s take a look.

Getting started

Every new git repository comes into the world with a set of predefined sample hooks.

$ git --version
git version 2.7.4
$ mkdir foo && cd foo
$ git init
$ ls -1 .git/hooks/
applypatch-msg.sample
commit-msg.sample
post-update.sample
pre-applypatch.sample
pre-commit.sample
prepare-commit-msg.sample
pre-push.sample
pre-rebase.sample
update.sample

Each hook is an executable shell script. In true git style, the batteries are included but optional: we’ll need to trim the trailing .sample from each hook’s filename to enable it.

Take pre-commit, which runs immediately before a commit would be created. The sample hook validates that new commits are free of whitespace errors (e.g. trailing newlines) and don’t depend on platform-dependent (non-ASCII) filenames. Let’s see it in action by moving pre-commit.sample to pre-commit and trying to commit a file with a whitespace error:

$ mv .git/hooks/pre-commit.sample .git/hooks/pre-commit
$ echo file with trailing newline > bar
$ echo >> bar
$ git add . && git commit -m 'Adds invalid file'
bar:2: new blank line at EOF.

In addition to the “blank line” warning, we can also see that the hook has exited with status 1. More importantly, the attempted commit was aborted:

$ echo $?
1
$ git show
fatal: your current branch 'master' does not have any commits yet

Just like that, git’s sample pre-commit hook is now protecting this repository against simple formatting errors.

The basics

Our quick tête-à-tête with the sample hook outlined the basics:

  1. hooks are implemented in predefined scripts in .git/hooks
  2. hook scripts must be executable
  3. hooks fail with non-zero exit statuses

Testing a hook

Hooks are just scripts. To test them, either by hand or through an automated testing system, all we need is a working git index and a shell that can run the hook. Testing the pre-commit sample against a clean index, for instance, we should see that everything is 'OK':

$ ./.git/hooks/pre-commit && echo 'OK'

For the handful of hooks that expect parameters, we can simply pass arguments in directly. commit-msg, for example, expects an in-progress commit message as its only argument:

$ echo 'Corrects a criminal complication' > /tmp/commit.msg
$ ./.git/hooks/commit-msg /tmp/commit.msg && echo 'OK'

In both cases the test recipe remains the same. Configure the git index and script parameters in a breaking scenario, run the hook, and assert a non-zero exit status. Fix the index, re-run the hook, and watch for a happy 0.

Tuning hooks

We’ve set up and tested hooks. They work; they’re right; and it’s time to make them fast. “Fast” may not be a concern for hooks like commit-msg, but sophisticated linting in a pre-commit hook can add up quickly in a larger project. Validation takes time, more validation takes more time, and it should go without saying that hooks shouldn’t block us when we’re trying to commit code. Lucky us! git and a garden-variety unix shell are quite capable of pruning a crowded git repository down to the relevant pieces.

First, we are usually only interested in validating what has changed. All we need to decide is what to compare to–a commit, branch, or other git ref. This could be HEAD (in a pre-commit hook, HEAD will point to the previous commit), a main branch like master, or anything else useful in the context of the project.

Once we have a basis for comparison, our hooks can invoke git diff --cached to pick through the change we’re about to commit. To simplify things further, we’ll only consider whole files, using --name-only to skip over the details within the diff.

$ echo 'hello world' > readme.txt && git add readme.txt
$ git diff --cached --name-only
readme.txt

Useful! Let’s alias it in a bash function for ease-of-use later on in the hook:

# within the hook
changed-files () {
  git diff --cached --name-only "$@"
}

Now we can do things like:

# later on, still within the hook
changed_css_files=$(changed-files *.css)

No more need to validate CSS style across the entire stylesheet directory–our hooks can now conditionally target only the files that have changed.

Managing hooks

As teams grow, workflow consistency is maintained by cutting and pasting hooks between repositories. This isn’t ideal, but since hooks live outside the index, each developer must manage their own copy. And since hooks’ changes aren’t tracked, small improvements made by one developer have no obvious path for delivery to others. We could contrive a workflow that would check the hooks in–probably something involving submodules, a monorepo, or at least some extra shell scripts around the edges–but odds are that the problem has already been solved!

Several stable, open-source projects would be happy to help manage git hooks on our behalf:

Conclusion

Git hooks are an unobtrusive, built-in tool for developing a consistent git workflow. Writing and testing them is easy–they’re just scripts!–and with just a bit of attention we can keep them running fast and true.



6/16/2016

How to Ruin Code Review

Code review is a powerful tool for ensuring quality, preempting bugs, and improving shared understanding. Spend enough time at it, and you’re guaranteed some wonderful moments: the satisfaction of a shared revelation, say, or the collective sigh of relief at a breaking change stopped short of production. You’re also assured at least a few stinkers.

At its best, peer review is a time for knowledge transfer, alignment, collaborative improvement, and team growth–but this story isn’t about that.

I promised a guide to crippling peer review, so here it is: a collection of simple strategies for inflicting misery on reviewer and author alike.

As the author

It’s your move! As you take one last look over the hard work you’re about to publish, consider the agony you’re about to inflict.

  1. Submit a monster. Besides the sheer time and energy it takes to comb through a large diff, large changes–or changes that attempt to tackle multiple issues in a single review–leave plenty of corners for defects to hide in. With the reviewer struggling to understand the scope of the change, whatever commentary they provide is likely to be as unfocused as the code it addresses–bad news for constructive code review, and bad news for quality overall.

  2. Don’t explain why. Whether delivering value to a customer, fixing a bug, or just scrubbing old code, every change is up to something. Hiding the raison d’être of any but the most trivial change will beguile your reviewer and leave them struggling once again to understand what they are supposed to be reviewing.

  3. Change things for change’s sake. Code is occasionally written without a reason to be. Submitting it for peer review–or including unused code in a broader, useful change–is a great way to clog the queue and distract from work that is actually adding value.

  4. Take it personally. Review is about the feature, the team, and the longterm sustainability of the codebase. Mistaking constructive criticism for a personal attack is a surefire way to start an argument, breed ill-will, and drain the value out of an otherwise productive review.

  5. Submit less than your best. Peer review isn’t development. When incomplete (or broken) code is submitted for review, it invariably ends up rewritten and resubmitted for a fresh round. Skipping local validation–both form and function–of a change’s behavior will frustrate the reviewer and waste everyone’s time.

As a reviewer

You have a few tricks of your own! Not to be outdone, you can easily sap your critique of any effectiveness it might otherwise have had. Just:

  1. Ignore the context. Dive into review before determining what you’re reviewing and the issue that it’s designed to resolve. This won’t just lead to confusing, off-topic commentary as you talk past the problem at hand–it will also ruin the author’s own thoughtful attempts to describe problem and solution in the structure and specs you are reviewing.

  2. Rewrite everything. It’s better to catch architectural flaws in review than an overhaul months down the line, but code review is rarely the right forum for tackling issues far upstream. Assuming the development process is in working order, the author has typically invested more research and consideration in their work than the reviewer. While the back-and-forth of reinventing their work may be good for education, it’s sure to halt the development process and distract from the review at hand.

  3. Pick nits. The sweet spot for code review lies somewhere between high-level design decisions and the sort of minutiae that are easily enforced through static analysis. Calling out formatting changes, entity names, and inline comments that don’t impact function can help delay the merge of otherwise useful code.

  4. Attack the author, not the code. Personal attacks will put the author on the defensive. Any energy invested in self-defense is energy unavailable for developing great software–and that’s not to mention the ill-will created ahead of the next review.

  5. Bring the dogma. Time marches. Context changes. Guidelines that were relevant one day may not matter the next–but adhering to them anyway can surely slow the pace of progress. Every change begs the question, “what’s changed around it?” Convention exists for a reason, but applying it inflexibly is both an easy way to resist change and a guarantee of eventual stagnation.

On the other hand

And there you have it–code review, without the good parts. Spend enough time at it, and you’ve seen it before. Pedantry, inflexibility, ignorance, and every excuse imaginable to keep a change from reaching production.

On the whole, I have yet to encounter a peer review process doing more harm than good. It’s an efficient, valuable tool for developing a team and its code.

Let’s end on a positive note. Inverting the “do-nots”, we can arrive at an equally relevant guide for encouraging expedient, positive code reviews.

As an author:

  1. Focus changes (one issue per review)
  2. Include context (reference issue numbers, write effective commit messages, add commentary for your reviewer)
  3. Avoid pointless or self-serving changes
  4. Use constructive criticism to grow
  5. Validate your own work (skim the code; verify its behavior)

And as a reviewer:

  1. Understand the context (supporting issues, previous code)
  2. Give the benefit of the doubt
  3. Don’t pick nits (computers are good at this)
  4. Review the code, not the author
  5. Be flexible

That’s it for now! What elements do you see in great code review (or in the reviews that you’d rather forget)? Share them on Twitter.

Further Reading



But wait! There's more—

View all posts