On Wed, Jun 1, 2011 at 4:06 PM, Brion Vibber <[email protected]> wrote: > "This isn't ready for core yet -- code looks a bit dense and we don't > understand some of what it's doing yet. Can you help explain why you coded > it this way and add some test cases?"
IMO, this is totally fine as a reason to reject a commit. It gives clear reasons why it's being rejected and suggests how to fix them. Another good reason, which I've given in the past to at least one thing I was asked to review on Bugzilla, is "This is doing too many things at once -- break it up into a bunch of smaller patches so I can review them individually." But just letting the patches languish with no reason is really bad, and reverting them with no reason once they were already committed is psychologically even worse (even though practically it's similar). _______________________________________________ Wikitech-l mailing list [email protected] https://lists.wikimedia.org/mailman/listinfo/wikitech-l
