On Wed, Jun 1, 2011 at 8:23 AM, Roan Kattouw roan.kattouw@gmail.com wrote:
This, for me, is the remaining problem with the 72-hour rule. If I happen to commit a SecurePoll change during a hackathon in Europe just as Tim leaves for the airport to go home, it's pretty much guaranteed that he won't be able to look at my commit within 72 hours. Or maybe I'm committing an UploadWizard change just after 5pm PDT on Friday, and the next Monday is a federal holiday like the 4th of July. Or maybe the one reviewer for a piece of code has just gone on vacation for a week and we're all screwed.
We could pick a different window, but I don't see how it can be much longer if we're also pursuing much more frequent deployments. If we're deploying every week, and something hasn't been reviewed at deploy time, the reviewer can either rush a review of code they may or may not really understand, or revert.
I think using the word "screwed" here belies the fundamental social problem we have with respect to reverts. As Brion has pointed out over and over, being reverted is not the end of the world. It doesn't mean "this code must die", it means "this code isn't ready for trunk yet". It still is in the repository and is available for comment and debate, and recommitting the code can happen a week or two later.
The genesis of the 72-hour idea was our discussion about what is stopping us from pre-commit review. The set of us in the office discussing this felt it was pretty much just a tools issue; from a policy point of view, we think it makes sense. Given that the 72-hour rule is less severe than pre-commit review, do you believe that our original premise is flawed (i.e. requiring pre-commit review is a bad idea)?
Rob