<mailto:
rupp@mcs.anl.gov>> wrote:
> Hi Matt,
>
>
>> Again, there are limits to everything, and this surpasses
the useful
>>
>> limit to this kind of specification. This is not personal
>> expression, this
>> is ease of reading.
>>
>>
>> Also, judging by the ENORMOUS number of source code changes,
"everyone"
>> was not following the informal guidelines.
>
>
> Yes, the changes are 'enormous' if you count the absolute number
of lines.
> However, my overall impression while refactoring was that I
touched ~1% of
> the total code base only. It's the huge size of the code base
that makes the
> number of changes appear to be high.
>
> Also, an estimated 50% of the violations stem from a copy&paste
of older
> code or previous examples rather than intentionally ignoring the
style
> guidelines. Thus, by bringing everything to a consistent state, a
(large)
> number of such copy&paste-induced violations are no longer going
to happen
> at all.
>
> The PETSc coding style guide needs to define exactly one style.
Your style
> preference obviously differs. My personal preference differs
slightly, too.
> Maybe Jed and/or others would prefer another slight variation.
Nevertheless,
> allowing a bit more flexibility in one's own personal coding style by
> following one common coding style is to the benefit of the whole
project.
> Visual consistency is an obvious one, but most importantly it aids in
> setting up other scripts for increased uniformity among the
different PETSc
> modules.
>
> You may argue that it does not matter whether we have 'for(...)'
or 'for
> (...)'. While this is certainly right for a compiler, I would
like to give
> you one example where it *does* make a difference:
> src/mat/interface/matrix.c:
> PetscErrorCode MatSetValuesAdifor(Mat mat,PetscInt nl,void *v)
> So, keeping consistency with the style guide, we would write
> for (...) {...}
> MatSetValuesAdifor(...) {...}
> and there is the additional blank contributing to the difference
between the
> function call and the for-loop. If, however, we have
> for(...) {...}
> MatSetValuesAdifor(...) {...}
> any scripts additionally need to check for characters in front of
'for'.
> Fortunately, it is *currently* sufficient to just consider all
characters
> other than a blank in front of 'for' to be a function call.
However, one day
> we might find code such as
> do_something(...);for(...) {...};
> or even
> cilk_for(...)
> and all of a sudden we are unable to use any simple checker
scripts any
> more. With a common PETSc style, the additional blank would still
allow for
> far simpler checks and refactoring if required.
>
> Just my 2 cents…
Thanks, Karl. I appreciate your effort on this front.