On 08/20/2013 11:08 AM, Daniel P. Berrange wrote: > On Tue, Aug 20, 2013 at 04:02:12PM +0100, Mark McLoughlin wrote: >> On Tue, 2013-08-20 at 11:26 +0100, Mark McLoughlin wrote: >>> On Thu, 2013-08-15 at 14:12 +1200, Robert Collins wrote: >>>> This may interest data-driven types here. >>>> >>>> https://www.ibm.com/developerworks/rational/library/11-proven-practices-for-peer-review/ >>>> >>>> Note specifically the citation of 200-400 lines as the knee of the review >>>> effectiveness curve: that's lower than I thought - I thought 200 was >>>> clearly fine - but no. >>> >>> The full study is here: >>> >>> http://support.smartbear.com/resources/cc/book/code-review-cisco-case-study.pdf >>> >>> This is an important subject and I'm glad folks are studying it, but I'm >>> sceptical about whether the "Defect density vs LOC" is going to help us >>> come up with better guidelines than we have already. >>> >>> Obviously, a metric like LOC hides some serious subtleties. Not all >>> changes are of equal complexity. We see massive refactoring patches >>> (like s/assertEquals/assertEqual/) that are actually safer than gnarly, >>> single-line, head-scratcher bug-fixes. The only way the report addresses >>> that issue with the underlying data is by eliding >10k LOC patches. >>> >>> The "one logical change per commit" is a more effective guideline than >>> any LOC based guideline: >>> >>> https://wiki.openstack.org/wiki/GitCommitMessages#Structural_split_of_changes >>> >>> IMHO, the number of distinct logical changes in a patch has a more >>> predictable impact on review effectiveness than the LOC metric. >> >> Wow, I didn't notice Joe had started to enforce that here: >> >> https://review.openstack.org/41695 >> >> and the exact example I mentioned above :) >> >> We should not enforce rules like this blindly. > > Agreed, lines of code is a particularly poor metric for evaluating > commits on.
Agreed, I would _strongly_ prefer no enforcement around LOC. It's just not the right metric to be looking at for a hard rule. -- Russell Bryant _______________________________________________ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
