Open Stack

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. 


Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|