[petsc-dev] I hate this one

[Milad Fatenejad]

Thanks Karl!

On Wed, Feb 13, 2013 at 10:38 AM, Karl Rupp <rupp at mcs.anl.gov> wrote:

> Hi Milad,
>
> the checks are in src/contrib/style/checks/. You can run them from
> PETSC_DIR, but they currently check the full source tree only. Compare with
> http://krupp.iue.tuwien.ac.at/**petsc-style/<http://krupp.iue.tuwien.ac.at/petsc-style/>
> to see whether you've introduced additional 'violations'. You can also
> generate the full HTML output by running src/contrib/style/runhtml.sh from
> PETSC_DIR and view/compare the output in src/contrib/style/html/index.**
> html
>
> Best regards,
> Karli
>
>
>
> On 02/13/2013 10:32 AM, Milad Fatenejad wrote:
>
>> Hello:
>>
>> I am working on modifying PETSc to interoperate with the MOAB mesh
>> library. I'd love to get some of Karl's scripts to ensure that my code
>> is compliant with the style guide. Are they available somewhere?
>>
>> Thank You
>> Milad
>>
>> On Mon, Jan 21, 2013 at 10:19 AM, Sean Farley <sean at mcs.anl.gov
>> <mailto:sean at mcs.anl.gov>> wrote:
>>
>>     On Mon, Jan 21, 2013 at 10:06 AM, Karl Rupp <rupp at mcs.anl.gov
>>     <mailto:rupp at 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.
>>
>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.mcs.anl.gov/pipermail/petsc-dev/attachments/20130213/18644485/attachment.html>