On Wed, Jun 1, 2011 at 11:23 AM, Roan Kattouw <roan.katt...@gmail.com> wrote: > On Wed, Jun 1, 2011 at 1:49 AM, Happy-melon <happy-me...@live.com> wrote: >> By far the most likely outcome of this is that in the two months following >> its introduction, 95% of all commits are reverted, because the people who >> are supposed to be reviewing them don't spend any more time than usual >> reviewing them. If I make a change to the preprocessor, HipHop, Sanitizer, >> SecurePoll, passwords, tokens, or any of a number of other things, I'm going >> to need Tim to review them. I don't begrudge Tim the thousand-and-one other >> things he has to do besides review my code within 72 hours. Does that mean >> that no one should make *any* changes to *any* of those areas? More >> dangerously still, does that mean that **only people who can persuade Tim to >> review** be allowed to make changes to those areas? I know what the answers >> to those questions are *supposed* to be, that's not the point. The point is >> **what actually happens**. >> > 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. > > So yes, before we implement this we need to 1) ensure that reviewers > spend enough time reviewing and 2) figure out what to do with > one-reviewer situations. >
I don't think "revert in 72 hours if its unreviewed" is a good idea. It just discourages people from contributing to areas in which we only have one reviewer looking at code. I *do* think we should enforce a 48hr "revert if broken" rule. If you can't be bothered to clean up your breakages in within 48 hours of putting your original patch in, it must not have been very important. -Chad _______________________________________________ Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
