[Insight-developers] Style in your gerrit review code

Johnson, Hans J hans-johnson at uiowa.edu
Tue Jun 14 10:47:04 EDT 2011


I would just like to caution that KWStyle is not always correct, and I've
found in the past that code changes
were made to accommodate seemingly arbitrary KWStyle behavior rather than
to enforce a consistent style.

The following is my understanding of the past years conversations about
this topic.

The discussion last year was that a consistent style that was enforceable
in some automated way be made available.  The inconsistent behavior of
what KWStyle complains about does not always follow a simple and straight
forward rule set.  This has lead to a complicated set of rules of when
KWStyle requires indenting.

I believe that this is a case where KWStyle's checking has been turned off
because it provides many false positive warnings about indenting that lead
to very convoluted rulesets, especially for nested namespace,
enumerations, or private classes.

Fixing KWStyle was going to be difficult and was a lower priority issue,
and disabling the quirky behavior of this one rule was a compromise.

Regards,
Hans
--
Hans J. Johnson, Ph.D.
hans-johnson at uiowa.edu
Assistant Professor of Psychiatry
University of Iowa Carver College of Medicine
W278 GH, 200 Hawkins Drive

Iowa City, Iowa 52242
Phone:  319-353-8587







-----Original Message-----
From: Bill Lorensen <bill.lorensen at gmail.com>
Date: Tue, 14 Jun 2011 10:17:21 -0400
To: "Lamb, Peter (GE Global Research)" <Peter.Lamb at ge.com>
Cc: ITK <insight-developers at itk.org>
Subject: Re: [Insight-developers] Style in your gerrit review code

Peter,

Is your KWStyle up-to-date?

Bill

On Tue, Jun 14, 2011 at 9:26 AM, Lamb, Peter (GE Global Research)
<Peter.Lamb at ge.com> wrote:
> Hi Bill,
>
> Many thanks for your e-mail.  I found that for some header files,
>KWStyle actually threw errors (for example, one line with a brace was
>indented only two but should be indented four), and for other files, no
>errors were thrown.  So I modified the files that KWStyle threw errors
>on.  But I'll go through and undo this particular type of change I made
>so everything is consistent with the style convention you've outlined
>below.
>
> Thanks again!
>
> Peter
>
> Peter Lamb
> GE Global Research, KW-C209
> Diagnostics & Biomedical Technologies
> Biomedical Image Analysis Lab
>
> T  +1 518 387 6024
> M +1 917 741 9233
> peter.lamb at ge.com
> www.gehealthcare.com
>
> One Research Circle
> Niskayuna, NY 12309 USA
> General Electric Company
>
> GE imagination at work
>
>
> -----Original Message-----
> From: Bill Lorensen [mailto:bill.lorensen at gmail.com]
> Sent: Tuesday, June 14, 2011 8:34 AM
> To: Lamb, Peter (GE Global Research)
> Cc: Insight Developers
> Subject: Style in your gerrit review code
>
> Peter,
>
> Thanks for reviewing the code in gerrit.
>
> I noticed that in .h files you are indenting the {}'s for method bodies.
>
> The indentation style was changed a while ago to be consistent with the
>indentation in the .txx files. Unfortunately, KWStyle seems to accept
>either.
>
> For example this original code in
> Core/ImageAdaptors/include/itkRGBToVectorPixelAccessor.h
>  inline void Set(InternalType & output, const ExternalType & input) const
>  {
>    output[0] = input[0];
>    output[1] = input[1];
>    output[2] = input[2];
>  }
>
> should NOT be changed to:
>  inline void Set(InternalType & output, const ExternalType & input) const
>    {
>    output[0] = input[0];
>    output[1] = input[1];
>    output[2] = input[2];
>    }
>
> Bill
>
_______________________________________________
Powered by www.kitware.com

Visit other Kitware open-source projects at
http://www.kitware.com/opensource/opensource.html

Kitware offers ITK Training Courses, for more information visit:
http://kitware.com/products/protraining.html

Please keep messages on-topic and check the ITK FAQ at:
http://www.itk.org/Wiki/ITK_FAQ

Follow this link to subscribe/unsubscribe:
http://www.itk.org/mailman/listinfo/insight-developers



________________________________
Notice: This UI Health Care e-mail (including attachments) is covered by the Electronic Communications Privacy Act, 18 U.S.C. 2510-2521, is confidential and may be legally privileged.  If you are not the intended recipient, you are hereby notified that any retention, dissemination, distribution, or copying of this communication is strictly prohibited.  Please reply to the sender that you have received the message in error, then delete it.  Thank you.
________________________________


More information about the Insight-developers mailing list