[Insight-developers] CMakeLists.txt manual uncrustify

[Hans Johnson]

Brad,

I agree with your experiences, and this leads to the fact that we should
have done this uncrustification even earlier than we did.  This is always an
issue with automatic code beautifiers, and is the primary reason that I
believe that uncrustify (or any code beuatifier) SHOULD NOT BE RUN
automatically on the code.

One possibility is to make a manual running of the code beautification tool
(and perhaps other automatic code manipulations) part of the release cycle.
Make a rule that a well defined set of tools can only be run immediately
AFTER a release is made.

Thanks for the feedback.

Hans



On 8/26/10 9:00 AM, "Bradley Lowekamp" <blowekamp at mail.nih.gov> wrote:

> Hello Hans,
> 
> Here is some negative feedback :)
> 
> At this point between the uncrustification of the ITK code and the proposed
> stylization of CMake files, no  non-trivial topic branch is going to be able
> to be cleanly merged that was fork before these changes. The biggest issue
> with this is any additional maintenance patches for the release branch, are
> not going to easily be applied to both the release and the master.
> Additionally, it took some time yesterday to rebase my topic branches to the
> uncrustified code, and git did make a couple merging errors. I suppose I am
> just gripping about that, and don't need further discussion :)
> 
> Unfortunately, durring my rebasing yesterday I could already see my code
> diverging from the uncrustified style. This is not good. Because we can not
> regularly run uncrustify on the whole repository for the above reason. Perhaps
> there really should be a better commit hook for this or something. Because if
> we don't enforce it, the code will diverge and this work will have been done
> in vain. 
> 
> 
> On the positive side:
> 
> I definitely this these changes are for the best.
> 
> Brad
> 
> On Aug 26, 2010, at 9:37 AM, Hans Johnson wrote:
> 
>> http://www.vtk.org/Bug/view.php?id=11175
>> 
>> The next step in removing end-of-lines is to run through all the
>> CMakeLists.txt files and clean them up.
>> 
>> I would like to propose that we also change all the key words to lower case.
>> The primary reason for this is that all documentation for CMakeLists.txt now
>> shows the key words as lower case.  Even the printed ³Mastering CMake v5²
>> uses lower case.  I¹ve come across this several times in my class where
>> students stumble over the fact that the documentation about Cmake does not
>> match the implementation in ITK.
>> 
>> I have a script (a wrapper around a vim macro) written that can make these
>> substituions reliably (Tested on the BRAINS tree, the Slicer3 tree, and the
>> ITK tree without causing any errors).
>> 
>> =====================
>> I¹ve discussed this with several people in the community, and have not yet
>> received any negative feedback (mostly just neutral feedback, and one adamant
>> positive feedback).
>> 
>> Provided that no negative feedback requesting further discussion is recieved:
>> My plan is to make these changes, test, and push to the repository before
>> next Tuesdays meeting.
>> 
>> Regards,
>> Hans

-- 
Hans J. Johnson, Ph.D.
Assistant Professor
200 Hawkins Drive
T205 BT, The University of Iowa
Iowa City, IA 52242

hans-johnson at uiowa.edu
PHONE: 319 353 8587

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.itk.org/mailman/private/insight-developers/attachments/20100826/756eb808/attachment.htm>