On Thu, 21 Feb 2008 09:54:05 +1100, Jonathan Lange <jml@mumak.net> wrote:
[snip]
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.
Yes, please.
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.")
Sounds great. Of much less significance, but related and most people probably don't know about it, "Reopens #1234" in the revert commit message will take care of adding the commit message to the ticket and re-opening it. Jean-Paul