CodingGuideLines: Add Link to Wiki

[ax3l]

@f-schmitt-zih @Heikman @ahelm @psychocoderHPC @PrometheusPi Please add your comments about the current draft in this pull request!

[f-schmitt]

• typo: "Indentation"
• I would prefer the following (space for comparisons):
  foo( param1, param2 )
  if ( param1, param2 )
• Template Arguments shall have descriptive names (i.e. not T or T_)

[ax3l]

thanks!
what we mean by Template Arguments: they should be descriptive, but only start with T_. They can/should most of the time be typedef'ed anyway (e.g. in classes) before they get used, so a ValueType or Option can change and the rest of the "implementation" only depends on that typdef at the beginning of the class.

[ax3l]

After a short talk: I personally prefer sticking the (...) without a space to the caller, since a space after if, for, foo( ... ) looks quite c-style (CastMe) otherGuy and one can see the guarded call arguments quite easy for long calls.
However we decide, I would handle method/function calls the same as if/for's since they are basically also a lambda like functor.

[psychocoderHPC]

Should we use one more line to define functions, because the return type of a function can be very complex e.g.

template<class T_A>  /* templates */
HDINLINE         /* compiler hints */
typename boost::result_of< typename OtherClass<T_A>::type >::type /*result type of function*/
functionName( ... )  /* name params */
{
  /* ... */
}

[ax3l]

We should also add a point about the conventions ValueType, resultOf/type (only cause of boost...) and others in template meta programming.

[psychocoderHPC]

To the Point with 2 or 4 spaces: I think we should switch to 2 spaces because of the strong template usage we had very long lines.

[ax3l]

... and due to a whole bunch of namespaces and sub-classes and stuff.
btw: if we go for two spaces, we could indent namespaces again :)

[psychocoderHPC]

We should add the rule that we not indent inside of namespaces

namespace Bob
{
struct Foo;
}
//wrong style
namespace Alice
{
    Struct FooWrong;
}

[f-schmitt]

I propose to use uncrustify as a pre-commit hook to ensure coding guidelines.

[ax3l]

Good idea, I though about vera++ so far, too.
@f-schmitt-zih Does uncrustify support transformations and user-defined rules?

[ax3l]

@psychocoderHPC namespaces:

I would prefer intenting namespaces (if standard spacing is 2 spaces).

Anyway, if not I would prefer to start intending with the first non-namespace object like this:

namespace Bob
{
/* no spaces */
namespace Alice
{
  /* this guy gets intended */
  Struct FooMe;
} // namespace Alice
} // namespace Bob

[f-schmitt]

+1 for @ax3l s comment

let's just indent (yes, it is written like this) just non-namespaces

[ax3l]

k, so no spaces for opening a new namespace. but every level inside them. added to wiki.

[ax3l]

Maybe the only exception for this rule?

namespace Bob   /* "general" outer namespace */
{
namespace Alice /* "general" outer namespace */
{
  /* this guy gets intended */
  Struct FooMe;

  /* some brainy likes to create a long mixed file
   * -> indent to avoid mixing it with the general outer namespaces */
  namespace SI
  {
    /* you should avoid that, ok in config files like .param ? */
  } // namespace SI

  /* this guy gets intended */
  Struct FooMeAgain;

} // namespace Alice
} // namespace Bob

But we could also skip that and stay as it is defined in the first place. Just looks a bit crude.

[PrometheusPi]

looks good to me 👍
Why must we not comment as written below (except that this is an inline comment)?

int var = 12.0; // the meaning of life

[psychocoderHPC]

for precompiler commands we should also use a nice style like this:

#if(VARIABLE_BOB==1)
#  if(VARIABLE_ALICE==1)
#    define HPC +1
#  endif
#endif

[ax3l]

👍 boost does the same, I like that, too.

Related to #143 : we should check if the file contains a recent copyright notice (if possible) and on a directory-basis if the right license header was added.

[ax3l]

@PrometheusPi

Why must we not comment as written below (except that this is an inline comment)?
int var = 12.0; // the meaning of life

Because that causes hard to track nvcc compile errors in cases like this:

void function( int a, // my first parameter
               int b
                   )
{
  /* ... */
}

Nvcc comments out the follow-up line(s) int b which sometimes still produces valid code but is definitely not what one intended. (_Disclaimer: can be "old" nvcc behaviour.)
Furthermore, /_ ... */ is more explicit (in the closing section) so one should prefer that.

[ax3l]

@ComputationalRadiationPhysics/picongpu-maintainers I really had to try that feature.
Any more comments?

[psychocoderHPC]

Can we use no white spaces between compare operators? Than we have a stronger binding of compare and other operators.

      if ( test == 1 && test2 == true )
      {
        y = y + -1;
      }

vs no spaces

      if ( test==1 && test2==true )
      {
        y = y + -1;
      }

[f-schmitt]

behold, I let's use parantheses instead!

if ( ( test == 1 ) && ( test2 == true ) )
{ }

[psychocoderHPC]

I think this is not possible with uncrustify.

[ax3l]

I think using spaces is just fine.
And yes, "you"™ should add parentheses :) and @f-schmitt-zih really needs a sarcasm-annotator™.

[psychocoderHPC]

Do we need a limitation of 80 character per line? At the moment we wrap all long lines in #161. Is someone developing our code on such a small console? We use often very long names for classes and have a lot of typedefs and templates this effects line breaks and unreadable code on many parts of the code.
I vote to set the 80 character limit to 120 or 140.

[ax3l]

80 characters would be nice, since we actually do 3-way-merges.
Example see:
http://blogs.webworks.com/lauren/files/2010/10/3_way_comparison_xsl.png

But I would just ignore that for now (no warning). We might add a hard limit to 160 characters or so?
Lets go for 120 or 140 max.

[psychocoderHPC]

I have tested 100 and 120, both looks good for me, more than 120 is to much.

[ax3l]

ok, so let us try maybe 100 for now?

[psychocoderHPC]

👍 for 100 character per line.

[PrometheusPi]

👍 100 characters

[ax3l]

Reordered the wiki page: grouped the rules in 5 useful sections for better orientation.

[ax3l]

@ComputationalRadiationPhysics/picongpu-maintainers Any more comments? Else I would just say we can merge that for now and discuss minor updates in uncrustify related pull-requests?

[bussmann]

Also zu MEINER Zeit haben wir nur 40 Zeichen gebraucht.

Am 27.01.2014 um 17:31 schrieb Richard Pausch [email protected]:

:+1 100 characters

—
Reply to this email directly or view it on GitHub.

[ax3l]

Your braille display did not support more output anyway

label if (logical something) then
         some statements
         goto label
      endif 

Edit: just realized that comment is in arbitrary levels inappropriate.
Edit2: believe it or not, it is suprisingly true - Braille displays wrap at character 40

[psychocoderHPC]

A IBM punch card has 80 columns http://en.m.wikipedia.org/wiki/Punched_card

[f-schmitt]

100 cols is fine, more is not. otherwise a kdiff3 merge is scrolling hell!

[f-schmitt]

I am not sure if certain agencies can properly parse this and use our comments against us if not written in ASCII English
^annotated using sarcAnot, v1.0

[ax3l]

👍 for sarcAnot, v1.0.

Btt:

@ComputationalRadiationPhysics/picongpu-maintainers Any more comments? Else I would just say we can merge that for now and discuss minor updates in uncrustify related pull-requests?

[f-schmitt]

I'm fine with the current state, no remarks so far.