[petsc-dev] I hate this one

[Barry Smith]

On Feb 13, 2013, at 10:45 AM, Milad Fatenejad <milad at uchicago.edu> wrote:

> 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.

   Karl will be modifying them to be able to run directly on single source files and subdirectories.

  Barry

> Compare with
> 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.
> 
>