Rob Lanphier wrote: > On Tue, May 31, 2011 at 10:35 AM, Neil Kandalgaonkar > <ne...@wikimedia.org> wrote: >> Are we all in deadlock or something? <snip> > > independent suggestions, but any of these might help: > 1. We say that a commit has some fixed window (e.g. 72 hours) to get > reviewed, or else it is subject to automatic reversion. This will > motivate committers to make sure they have a reviewer lined up, and > make it clear that, if their code gets reverted, it's nothing > personal...it's just our process. > 2. We encourage committers to identify who will be reviewing their > code as part of their commit comment. That way, we have an identified > person who has license to revert if they don't understand the code. >
Russell N. Nelson - rnnelson wrote: > Worse than pre-commit review. What if you make a change, I see it, > and make changes to my code using your changes. 72 hours later, they > get reverted, which screws my code. Okay, so then nobody's going to > compile anything, or use anything, if it has 72-hour code in it. The > alternative, if they want the code badly enough, is to review it so > it will stick. Well, that devolves to the same thing as pre-commit > review. > > And ... this only makes sense for core, or maybe for stable > extensions. It doesn't make sense for experimental extensions where > only one person is likely to understand or care what the code says. Perhaps a combination is possible, a bit like Brion did in the old days: * A revision has to be reviewed within reasonble time (time upto it still allows reverting without pain, ie. hours or days. Not weeks! No more than, say, 3 days) * The people who do code review, which should always start at the back, will see soon enough if there's a revision stuck in the process (ie. not being reviewed / passed to someone else) at that point someone has to go bold and revert it. Nothing personal: If nobody understood your commit, either splits it up and document it better – or poke whoever is capable of reviewing your code and make sure he or she does so in time. We are responsible as a team (nothign like primary school hand-in-hand pairs of buddies, but you get the idea..). * A fixme should not maintain it's status for more than 24 hours. Not fixed within 24 hours ? Revert, no questions asked! * Once a week, on Monday, we make sure anything from after Friday noon (remember, 3 days!) is unreviewed or fixme'ed. They shall be reverted or sorted out the same day (some gnomes may want to do this over the weekend). * Tuesday anything last week should no longer be on our minds. We've moved on! "Tomorrow" (wednesday) is already the 3rd day of the week. Aside from the in-svn proccess, some points I learned from other committers and just now via IRC: * Unless the committer is subscribed to mediawiki-cvs or keeps a tight eye on Special:Code, one should have wiki-account linked in the CodeReview extension and e-mail notifications on (so you know about questions/comments., status changed and follow-ups as soon as possible) * the "todo" tag (or soon-to-be-introduced status "needsmore" or "incomplete")[1] does not mean "in another life time". * If you don't know where to start... http://etherpad.wikimedia.org/CodeReviewTracker -- Krinkle [1] https://bugzilla.wikimedia.org/28274 _______________________________________________ Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
