[Insight-developers] uncrustify git hook

[Matthew McCormick (thewtex)]

Hans and Brad,

Thank you for the feedback.

My impression was that the style changes had not been finalized, so feedback
was offered.  Since KWStyle did not address that issue, 'ignore' may be a
better route for now as suggested.  I pushed a change here:
http://github.com/thewtex/ITK/commit/7fcdbbd1c1577e44a39faf02438deba0b0292156

Perhaps a more scientific way to address such potentially contentious
specifications would count changes required for setting 'add' and 'remove'
and the changes required with 'ignore' for a percent change calculation.
 Then, the setting with the least changes required could be used or 'ignore'
if it is highly contentious or close.   I can work on a script to do this.

Brad's suggestion for setting up the style changes is good, and I can work
on that method.  I will try to get to it tomorrow.  The current list of
allowed KWStyle exceptions should also be incorporated.

The purpose of the proposed hooks is not to apply uncrustify incrementally,
but to allow for more agile development that will not break the KWStyle
test.  The workflow looks like this

- Changes are made concentrating on getting the code functional without
worrying about style.
- On commit, uncrustify automatically makes the code KWStyle compliant.
 Using a merge tool of choice, only uncrustify changes in the changed code
should be accepted.  As Hans remarks, there are also some bugs in uncrustify
for which monitoring is required.
- KWStyle is executed only on the changed code.  This prevents code that
breaks KWStyle from being committed and saves time relative to running the
KWStyle CTest on all the code.
- make and possibly ctest should be run after the commit to ensure nothing
broke.  git commit --amend can be use to fix it before any pulling or
pushing occurs.

Matt

On Fri, Sep 10, 2010 at 6:56 AM, Brad King <brad.king at kitware.com> wrote:

> On 09/09/2010 05:10 PM, Matthew McCormick (thewtex) wrote:
> > I drafted a git pre-commit hook to run uncrustify and KWStyle checks on
> > changes to the code.
>
> I agree with Hans that it makes more sense to run this before releases.
> People can rebase their unfinished topics on the release and update them
> to the new style.  Running this on every commit can cause lots of extra
> changes to a file and obscure the intended change.  We want to keep
> commits as granular as possible.
>
> A purely technical issue is that the "hooks" branch is actually a project
> shared by a bunch of projects.  It should have no project-specific files.
> Both uncrustify.conf and ITK.kws.xml.in should be in ITK's source tree.
> We can point at them in the configuration:
>
>  git config hooks.uncrustify.conf Utilities/.../uncrustify.conf
>  git config hooks.KWStyle.conf Utilities/KWStyle/ITK.kws.xml.in
>
> This can be simplified with a "Utilities/hooks.sh" setup script in the
> ITK source tree (which can also set up the .git/hooks in the first place).
>
> -Brad
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.itk.org/mailman/private/insight-developers/attachments/20100910/9ca79342/attachment.htm>