[Insight-developers] Re: Review Directory : Coding Style Problems

Julien Jomier julien.jomier at kitware.com
Thu Jan 25 13:48:27 EST 2007


Bill,

I strongly recommend to use the cvs version of KWStyle.
I'm going to cute a new release at the end of the month.

Also, I'm going to add KWStyle as part of the testing environment on the 
Insight Journal, that should help with the integration of new classes.

I agree that adding KWStyle as a cvs commit filter should not be done 
before we fix the current style issues, otherwise fixing a simple bug 
will take for ever...

Julien

Lorensen, William E (GE, Research) wrote:
> Stephen,
> 
> As an exercise, you should fix some files, .e.g. Code/SpatialObjects.
> Almost every file fails the style checker.
> 
> Also, others that vote for the commit check, who have not repaired,
> let's say 10 or so files, should pick some to try.
> 
> BTW, if you download the KWStyle1.0 version for windos, you'll see that
> there are some errors that would need to be fixed befroe we can impose a
> commit check.
> 
> One example: for every file I get an error like this:
> Error #11 (18) __itkImageMaskSpatialObject_h v.s.
> __tkImageMaskSpatialObject_h
> 
> Bill
> 
> -----Original Message-----
> From: Stephen Aylward [mailto:stephen.aylward at gmail.com] On Behalf Of
> Stephen R. Aylward
> Sent: Thursday, January 25, 2007 1:22 PM
> To: Lorensen, William E (GE, Research)
> Cc: Luis Ibanez; ITK
> Subject: Re: [Insight-developers] Re: Review Directory : Coding Style
> Problems
> 
> I think we need to identify one or two hard examples where you want to
> allow a longer line, discuss them, and then possibly put those on the
> wiki and in the style document.
> 
> KWStyle could probably check for variable names greater than
> (72-indentingLevel) chars.  Otherwise, I think we need to enforse the
> line breaks.  I use VIM, and the long lines kill the visualization of
> indenting/flow.
> 
> s
> 
> Lorensen, William E (GE, Research) wrote:
>> I agree with Gaetan. We should cleanup ITK first. This will also give 
>> folks an idea of how difficult it is to do. I have fixed dozens of 
>> files and it is time consuming and the long line restriction can lead 
>> to hard to read code occasionally (not frequently).
>>
>> Bill
>>
>> -----Original Message-----
>> From: insight-developers-bounces+lorensen=crd.ge.com at itk.org
>> [mailto:insight-developers-bounces+lorensen=crd.ge.com at itk.org] On 
>> Behalf Of Luis Ibanez
>> Sent: Thursday, January 25, 2007 12:20 PM
>> To: Stephen R. Aylward
>> Cc: ITK
>> Subject: Re: [Insight-developers] Re: Review Directory : Coding Style 
>> Problems
>>
>>
>> I agree,
>> we should run KWStyle as a CVS commit filter.
>>
>> That will simplify all the process.
>>
>>
>>    Luis
>>
>>
>> --------------------------
>> Stephen R. Aylward wrote:
>>
>>> My vote is a cvs commit check.   I think that is a great idea.
>>>
>>> Full disclosure - it does mean that some people wanting to fix one 
>>> line in a filter will need to change multiple lines in the filter in 
>>> order to commit their fix - since many filters already have lines that
>> are too long.
>>
>>> It really isn't that painful of a process, and we aren't the only ones
>>
>>> doing this.  KDE is adopting KWStyle checker - using KWStyle reduces 
>>> the learning curve for a library.  Consistent line lengths helps with 
>>> rapid
>>> (human) code parsing.
>>>
>>> Stephen
>>>
>>> Bill Hoffman wrote:
>>>
>>>
>>>> Gaetan Lehmann wrote:
>>>>
>>>>
>>>>> In the files I have fixed this morning, at least 95% of the problems
>>
>>>>> was the length of the lines. Like you, I would really prefer 
>>>>> spending this time doing things more useful.
>>>>>
>>>>> - that's why I ask again if the line length limit can be increased,
>>
>>>>> or removed
>>>>
>>>>
>>>> Like it or not, the line lengths are here to stay, so the best 
>>>> approach would be to write the code with the correct line length to 
>>>> begin with.  Many editors can be setup to flag or notify you that the
>>>> line length is past the limit.   Another approach, might be to make 
>>>> this a cvs commit check. That way the code never makes it to the 
>>>> dashboard with the long lines.
>>>>
>>>> -Bill
>>>>
>>>>
>>>> _______________________________________________
>>>> Insight-developers mailing list
>>>> Insight-developers at itk.org
>>>> http://www.itk.org/mailman/listinfo/insight-developers
>>>>
>> _______________________________________________
>> Insight-developers mailing list
>> Insight-developers at itk.org
>> http://www.itk.org/mailman/listinfo/insight-developers
>>
> 
> --
> =============================================================
> Stephen R. Aylward, Ph.D.
> Chief Medical Scientist
> Kitware, Inc. - Chapel Hill Office
> http://www.kitware.com
> Phone: (518)371-3971 x300
> _______________________________________________
> Insight-developers mailing list
> Insight-developers at itk.org
> http://www.itk.org/mailman/listinfo/insight-developers
> 



More information about the Insight-developers mailing list