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…
I could be wrong but I think that windows users won’t be able to use that hook.. I believe Hg under MozBuild is still the default win build of Hg, meaning it uses the windows shell not MSYS shell to execute this stuff.
Secondly for a “buggy patch” pre-hook thats both hard and easy.
Just cause it to build both debug and release Firefox, run full set of tests [to see if they pass], then check if any other commits happend since then, [re-]merge and push! Probably not useful, due to turn-around-time though
[I expect people interested in using this could make any necessary changes, and to be frank, I don’t care much about Windows here because I don’t use it (for pushes, or for much of anything else, to be honest). 😛 That said, when I installed the build toolchain on Windows a few days ago, the start-* scripts were spawning a bash shell, and the Mercurial there was responding to an hgrc in ~/.hgrc, so I’m not so sure this doesn’t work there.
“Buggy patch” was a joke about a hook that would be intelligent enough to find any bug, not just ones that would show up in automated testing runs — solving-the-halting-problem sort of bugs, that is.]
Comment by Justin Wood (Callek) — 10.01.09 @ 22:31
I guess Windows users can just put “c:\mozilla-build\msys\bin\bash.exe” in front of the script name.
But rather than searching for this specific string to be there I would be looking for /\br=\w+/ not being there. You can use whatever string to indicate that the patch isn’t reviewed but a reviewed patch will always have “r=something” in the commit string.
[That was intentional, because sometimes I’ll want to commit with something like “Fix typo to kick tinderboxen into doing another build” as the description (or some other such thing without adding a r= somewhere). Also, Mozilla’s style of indicating reviews probably doesn’t extend to other open source projects that use Mercurial (or, for that matter, personal on-disk repositories).]
Comment by Wladimir Palant — 11.01.09 @ 12:02