[Twisted-Python] How to say "reverted". (was Re: [Twisted-commits] r22628 - Revert r22624: regression in test_conch.)

[Thomas Hervé]

> Hello all,
>
> Thanks to Thomas for being alert enough to spot this regression and
> energetic enough to revert it. I'm very glad that we have people[1]
> monitoring the test suite making sure that we can trust trunk.
> However, I have two suggestions on how we can announce trunk reverts
> better. I'm not picking on him in particular, this is just the first
> revert for which I am detached enough to comment sensibly.
>
> 1. When reverting a commit to trunk, the commit message should explain
> what the regression is.
>
> The word 'regression' is used sometimes to mean 'test suite failure'
> and other times to mean 'a feature that I liked works differently now'
> or 'this is slower than it was'. If it's a test failure, it's useful
> to know what test, and particularly whether or not the test was
> related to the change. If it's not a test failure, it's good to know
> why the "regression" is considered severe enough to back out the
> change, rather than just fixing it in place.
>
> 2. Reverting someone's contribution is bad news for them. We should
> break the bad news gently.
>
> Backing out someone's changes can often send an unintended message of
> blame, when we actually want to be encouraging people to contribute.
> "Revert <revno>: regression in <file>." is terse, unspecific and
> leaves too much unsaid. We can't do anything about the bad news, but
> we can change the way we break it. Being more specific helps a lot, as
> does describing what happens next (e.g. "I'll fix it up and land it
> for you", "Can you please investigate the failure and fix the test,
> I'll review the fix for you as soon as it's ready.")

Thanks for insisting on that. I totally agree with you, but of course lazyness 
sometimes comes in the middle of the way. FWIW, I've contacted Paul before 
the revert, so I hope he did take it personaly :). As a matter of fact, I 
think the responsability of revert is the one of the reviewer, because it's 
the reviewer who gave an early go for the commit.

Thanks also to JP for the 'Reopens' feature. I've documented it on the 
ReviewProcess page, don't hesitate to modify it.

> [1] I'll be even happier when we have *machines* monitoring the test
> suite, but that's another email.

Currently, our test suite is a little bit too unreliable to do this 
(aka 'intermittent failures'). But maybe in a near future...

-- 
Thomas