10.01.09

How to not commit explicitly unreviewed changes in Mercurial

Tags: , , , , , — Jeff @ 21:45

Earlier today I made one of the most, er, special commits to Mozilla code that I’ve ever made — entirely due to the commit message I used:

Bug 466905 - Fix JSOP_NEWARRAY to be not-buggy and use it when possible. NOT REVIEWED YET

If you read bug 466905 you’ll see in comment 20 that the patch was reviewed by Brendan and was committed with the requested changes. So why is “NOT REVIEWED YET” in there? It’s an artifact of how I manage patches-in-progress; I assign the description when I create the patch, and since I can’t know whose review will eventually grace the patch, I just add a note that I’ll see when I review the change just before commit, and I make sure to fix the description immediately before pushing it into the main repository. What happened here is that I forgot to review the commit message for proper reviewed-ness.

<firebot> Check-in: http://hg.mozilla.org/tracemonkey/rev/6475993319c4 - Jeff Walden - Bug 466905 - Fix JSOP_NEWARRAY to be not-buggy and use it when possible. NOT REVIEWED YET
<Waldo> aargh
<shaver> sounds like someone needs a pre-push local hook!
<Waldo> quite possibly!

There’s a clear flaw in this process: the bad-commit-message check (and remembering to do it!) is done manually and can easily be forgotten. So, at shaver‘s suggestion, I dove into the world of Mercurial hooks. After a little reading from chapter 10 and section 11.3, I present you with ensure-not-unreviewed:

To use, simply drop that somewhere on your system, chmod +x it, and copy the following lines into ~/.hgrc (.hg/hgrc if you want this configurable on a per-repository basis):

[hooks]
preoutgoing.ensure_not_unreviewed = /path/where/you/downloaded/ensure-not-unreviewed

That should take care of the problem of buggy descriptions permanently. Now if only I could write a preoutgoing hook to prevent pushing buggy patches