[Insight-developers] Fwd: uncrustify git hook

[Matthew McCormick (thewtex)]

List bounced message because of size, so forwarding without results
attachment and image link here:
http://img375.imageshack.us/img375/9766/consensushistogram.png

---------- Forwarded message ----------
From: Matthew McCormick (thewtex) <matt at mmmccormick.com>
Date: Mon, Sep 13, 2010 at 1:29 AM
Subject: Re: uncrustify git hook
To: Brad King <brad.king at kitware.com>
Cc: ITK <insight-developers at itk.org>, Hans Johnson <hans-johnson at uiowa.edu>,
Luis Ibanez <luis.ibanez at kitware.com>


Hans, Brad, and ITKers,

A script as recommended by Brad that sets up the git commit hooks has been
uploaded here:
http://review.source.kitware.com/#change,70

ITK-Gerrit does not seem to have a hooks branch, so a cleaned up version of
the proposed changes to the pre-commit hook is here
http://github.com/thewtex/ITK/commit/ac1bce2dc068491028b4d90f77e38d521b9155d6

An uncrustify.conf based off Hans/Luis's wiki file was updated to version
0.56 and some of the values were empirically generated.  The method applied
was based on the ancient maxim "Thou who writeth the code should maketh the
rule."  Luckily, we have a 1+ million line dataset of code to perform
analysis on.  The script used to perform the analysis along with the
resulting raw data is attached.  The resulting configuration file can be
found here:
http://review.source.kitware.com/#change,71

Three types of uncrustify options are examined:
  boolean (false/true)
  ignore/add/remove/force
  ignore/lead/trail

For the boolean options, the configuration file is set to each value, and
the number of line changes required to enforce the value are counted.  The
value (false or true) with the lowest number of line changes is used.

A similar process is applied with the 'add/remove' and 'lead/trail' options.
 However, we also calculate a 'percent consensus' for these options by
examining the number of line changes required when the value is set to
'ignore'.  If we take 'a' to be the number and line changes for the 'add'
value, 'r' for 'remove', and 'i' for 'ignore', the consensus percent is

  consensus_percent = 100.0 * ( 1.0 - min( a-i, r-i ) / ( a-i + r-i ))

For these options we require the consensus percent to be above a threshold,
or the value is set to 'ignore'.

Tradition suggests a 2/3's majority, 66.7% consensus, should be used.
 However, if we look at a histogram of the consensus percent values
(attached), many more options can be specified by lowering the threshold.
 So, 57% is used.

My limited experience so far has been that an uncrustify will make a file
pass KWStyle test without problem apart from higher level issues like header
guards or missing namespaces.  Again, the purpose is speed up development
while retaining decent looking code.  As Hans and Brad remark, allowing
changes that are only style-related will make version history messy.  When
using the merge tool, the default merge file does not have any uncrustify
changes.  Only changes related to the commit should be brought in.  Commits
should be stylish content as opposed to style only.  Think The Artist
Formerly Known as Prince as opposed to Lady Gaga.  Or something like that
;-).

Matt
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.itk.org/mailman/private/insight-developers/attachments/20100913/6d2e68a0/attachment.htm>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: determine_uncrustify_conf.py
Type: application/octet-stream
Size: 6214 bytes
Desc: not available
URL: <http://www.itk.org/mailman/private/insight-developers/attachments/20100913/6d2e68a0/attachment.obj>