+1
I believe the way forward involves using pre-commit review, requiring test coverage to pass review, and developers working in branches at all times. SVN may be a pita when it comes to branches, but that's a solvable problem.
- Trevor
On Wed, Jun 1, 2011 at 10:52 AM, Andrew Garrett agarrett@wikimedia.orgwrote:
On Thu, Jun 2, 2011 at 1:28 AM, Chad innocentkiller@gmail.com wrote:
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.
I'm really not a fan of drop-dead deadlines in the one to two day range in general. I occasionally have periods of about two or three days when I don't have access to a computer (or time to use one).
I think that if we actually want pre-commit review (which I don't have a problem with), we should have pre-commit review instead of excessive reversion. Reverts make the revision log hard to follow, feel like a slap in the face to many developers (especially new ones, who this policy is supposed to be attracting!), and of course give us lots of merge conflicts and what not.
I think it would combine with commits of code that's broken in the first place to exacerbate the current situation where a single change can have up to ten associated revisions where people fix little things, revert, unrevert, and generally make things difficult to review and follow.
-- Andrew Garrett Wikimedia Foundation agarrett@wikimedia.org
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l