[Twisted-Python] Re: [Twisted-commits] r17325 - merge sob-491, fixes #491

[glyph at divmod.com]

On Thu, 22 Jun 2006 12:31:59 +1000, Andrew Bennetts <andrew-twisted at puzzling.org> wrote:
>On Wed, Jun 21, 2006 at 12:15:03PM -0400, Jean-Paul Calderone wrote:
>[...]
>>
>> To clarify, all commits to trunk should take on this form:
>
>Here's an idea: can we configure SVN to enforce this?

I've received this suggestion a few times privately and I'm still mulling it over.  On the face of it, it's a good idea, buuuuuut:

 1. I've had a series of disastrous experiences with commit hooks rejecting changes.  The guy who wrote the commit hook goes on vacation for 3 days, there's a bug in it, and nobody can figure out how to merge code or turn it off for 8 hours; meanwhile the guy trying to commit in bangalore goes to sleep, then doesn't show up again for a week, and the changes are lost.  (Having changes on a branch prior to trunk would alleviate this somewhat, I think, but it would be just as much of a pain to be blocked on some stupid merge-script bug.)
 2. The most important thing, really, is a sufficiently descriptive explanation for the change.  Any programmatic attempt to enforce this is sure to drive artificial gaming of the metric; i.e. if we require 3 sentences, "fixed random junk in foo module" will become "Fixed random. Junk in. Foo module."
 3. 99% of the time (e.g. all "normal" commits) should be in this format; however, the underlying tools, to be honest, are still kind of primitive, and we still have to leave a little wiggle room to deal with their foibles.  At least it isn't the masochistic chewing-gum-and-duct-tape perpetual catastrophe of CVS, but SVN still does annoying things like screwing up merges.  If there is a commit with a nice long description and a 'fixes' and everything, which needs to be reverted and reapplied for some dumb technical reason, I'd like to see

  r1234: "author: foo; reviewer: bar; wonderful wonderful description ..."
  r1235: "crap, reverting r1234 because of svn bug #9813741"
  r1236: "reapplying r1234 merged in utterly retarded way to work around svn bug #9813741"

not the entire description of r1234 repeated twice -- or, more realistically, three times, since the revert in the middle would also have to have a conformant commit message.
 4. precommit hooks keep the SVN SSH transport alive to report errors to the client for the duration of the hook.  When I last investigated (although this was in an SVN pre-release, a good 2+ years ago) if the connection drops while you're validating the commit message, it rejects the commit.  This would be especially annoying if you were committing an important fix over some intermittent link, like a GPRS modem.  That's not a common occurrance, but then again, if something were important enough that someone needed to commit code on a *GPRS modem* that is probably the time they would _least_ like something technical to go wrong with the process.


This is basically just the same reason that, despite our hardcore attitude towards tests, we don't have buildbot automatically reject or revert commits based on automated test failures.  The tools (in that case, buildbot, trial, and our test suite) are too primitive to rely on without some level of human judgement.  We've been moving to a stricter model so that the human judgements can be as mechanical and apolitical as possible (e.g. "was this a test failure due to too much load on the buildbot / known intermittent failure", not "is this an important enough area of code to worry about test failures in") but we still need it there.

I am eager to be corrected however; the more work our tools can reliably do for us the better.  Has anybody had a good experience with automatically-rejected commits in another reasonably sized project before?