Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Initial Discussion #2

Open
psychocoderHPC opened this issue Nov 24, 2015 · 12 comments
Open

Initial Discussion #2

psychocoderHPC opened this issue Nov 24, 2015 · 12 comments

Comments

@psychocoderHPC
Copy link
Member

psychocoderHPC commented Nov 24, 2015

This is the discussion to the first CRP C++ coding style

The style is a mix of both sources. I hope that I not lost rules during the reordering.

Goals:

  • create a coding style which can used for all C++ projects in ComputationalRadiationPhysics
  • the code should looks equal independent of the used editor, IDE ( incl. web based editors)
  • it must be write/ read able without IDE features such as
    • block collapse function
    • code highlighting
    • ...
  • break the 80 column limit should avoid as best as possible by design (new line rules)
  • rules should be consistent ( to add linter or code beautifier support )
  • with heavy template usage the code should stay readable
@psychocoderHPC
Copy link
Member Author

psychocoderHPC commented Nov 24, 2015

Why is this used in the draft and not that?

  • 80 characters as max line length limit
    • most MAC/Linux terminals open by default an 80 column console
    • a 3 way merge must fit a screen
    • avoid vertical scrolling during reviews on GitHub
      • Web "unified" ~100 chars is ok with most resolutions
      • Web "split view" ~60 chars
      • mobile? way less then 80
  • doxygen commands @ or \
    • with google I cant find any pro or cons for on of the versions
    • after checking the documentation I found that two command s @{ and @} can only be used with @
    • -> @ is used that all doxygen command are consistent
  • space vs. tab for indention
  • -> space is used that the code looks equal in all editors, browsers and IDEs
  • different rules for definitions and expressions
    • -> expressions get own rules to avoid lines of codes explosion
    • -> equations look better
  • why use not // for all comments
    • -> // are single line comments therefore it is used for single line comments
    • -> /* */ are multi line comments therefore it is used for multi line comments

@psychocoderHPC
Copy link
Member Author

@ComputationalRadiationPhysics/xrt-maintainers @ComputationalRadiationPhysics/dsp-maintainer @ComputationalRadiationPhysics/alpaka-maintainers @ComputationalRadiationPhysics/picongpu-maintainers
please enter this discussion.

@ax3l
Copy link
Member

ax3l commented Nov 24, 2015

the head of the PIConGPU wiki has some more open points regarding conventions specifically for meta programming and traits that we might want to covered at some point.

  • Add something template meta programming specific: public typedefs like ValueType and type (boost resultOf)
    • ValueType is a public typedef for internal storage types (e.g. a single element of a field)
    • type traits should either define a ::result or ::type (the literature is not very unified in that)
    • what is necessary for a resultOf (see also: this current problem)

@BenjaminW3
Copy link
Member

Should all discussions be within this thread (confusing) or should we open one issue per topic?
I want to add to point 7 that all Preprocessor Macros have to be prefixed with a project identifier to avoid duplications across projects. E.g. in alpaka all macros start with ALPAKA_.

@psychocoderHPC
Copy link
Member Author

IMO a pull request currently to master is the best.

@ax3l
Copy link
Member

ax3l commented Nov 24, 2015 via email

@psychocoderHPC
Copy link
Member Author

Note to myself, file rule missed:
a single empty line is the last line in a file

@Flamefire
Copy link

In https://github.com/ComputationalRadiationPhysics/contributing/blob/master/codingGuideLines/cpp.md#17-template-class--struct-and-type-definitions
What is the meaning of

for C++11 `using` new line after assign `=` mandatory ( definition is indented )

? The example does not help to understand that and the usage of the inline code tags seems to be odd.

Similar in https://github.com/ComputationalRadiationPhysics/contributing/blob/master/codingGuideLines/cpp.md#12-function-and-method-definitions with the discussion of the oc-tokens (what is that?) The description talks about new lines around parentheses (?) but there are none in the example. Maybe it is meant, that one can put arguments on own lines, in which case the first newline must be after ( and the last before )

@ax3l
Copy link
Member

ax3l commented Nov 25, 2015

that rule is removed, @psychocoderHPC pls remove the inconsistencies where it is still noted

oc tokens are defined in line section 1 (Namings), line 1

@ax3l
Copy link
Member

ax3l commented Nov 25, 2015

pls open new issues per topic!

@ax3l ax3l added the C++ Style label Nov 25, 2015
@psychocoderHPC
Copy link
Member Author

Please read this to see why we need a defined coding style:
http://www.smashingmagazine.com/2012/10/why-coding-style-matters/

@j-stephan
Copy link
Collaborator

after checking the documentation I found that two command s @{ and @} can only be used with @

This is not true (or it may have changed in the past 5 1/2 years). I'm using \{ in bactria and it works as expected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants