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

Formating tools #33

Open
psychocoderHPC opened this issue Nov 20, 2018 · 16 comments
Open

Formating tools #33

psychocoderHPC opened this issue Nov 20, 2018 · 16 comments
Labels

Comments

@psychocoderHPC
Copy link
Member

Here you can add tools those help to format the source code.
The goal is that we find a formatting tool to automate the formation.

@psychocoderHPC
Copy link
Member Author

psychocoderHPC commented Nov 20, 2018

tool: clang format

Online tool to configure the style: https://zed0.co.uk/clang-format-configurator/#
pull request: #32

@psychocoderHPC
Copy link
Member Author

psychocoderHPC commented May 14, 2020

Let me bring up this discussion and push the usage of #32.
We also have a style file suggestion in

IMO the most important question is, "Should we stop relaying on the nice template rules to be able to use clang format?"
Our current style guide: https://github.com/ComputationalRadiationPhysics/contributing/blob/master/codingGuideLines/cpp.md

The goal should be that we come soon to a automated formation because we have more and more distributed developer and a hand full projects with different styles.

@sbastrakov @ax3l @jkelling @SimeonEhrig @bernhardmgruber @BenjaminW3 @bertwesarg @j-stephan @ComputationalRadiationPhysics/alpaka-developers @ComputationalRadiationPhysics/picongpu-developers

Please answer the question: Do we should get rid of all formatting rules we can not apply with clang-format to provide a style file for all our projects.

@psychocoderHPC
Copy link
Member Author

Do we should get rid of all formatting rules we can not apply with clang-format to provide a style file for all our projects.

IMO yes we should.

Reasons are:

  • it is not possible to maintain all projects with different style
  • new developer will have it easier to contribute

@BenjaminW3
Copy link
Member

Yes. I already proposed one for alpaka.

@pordyna
Copy link
Member

pordyna commented May 14, 2020

I think that would make a lot of sense.

@jkelling
Copy link
Member

Yes.

@bernhardmgruber
Copy link
Collaborator

Yes, we should use an automated formatting tool and clang-format seems to be the dominant choice here.

As for the guidelines that cannot be enforced using clang-format: you can still keep them and apply them once clang-format supports the corresponding options. In the interim, we could mark the rules as "currently not enforceable but we wish we could have it".

@SimeonEhrig
Copy link
Contributor

By default my Emacs runs clang-format after each save, so: Yes

@BenjaminW3
Copy link
Member

We have to keep in mind that we do not only agree on a specific set of clang-format rules but also on the specific clang-format version because each version produces slightly different results and newer versions support more features.

I would vote for clang-format 10 😉

@sbastrakov
Copy link
Member

I also think we should be using clang-format.

@j-stephan
Copy link
Collaborator

I agree!

@bernhardmgruber
Copy link
Collaborator

As a sidenote: the template formatting proposed by the style guide seemed fairly easy to implement in clang-format. Here is a diff: llvm/llvm-project@master...bernhardmgruber:format

@psychocoderHPC
Copy link
Member Author

As a sidenote: the template formatting proposed by the style guide seemed fairly easy to implement in clang-format. Here is a diff: llvm/[email protected]:format

If you can add an option in clang format for that and push a PR to clang/llvm it would be wonderful. Until it goes mainline we can aggree on e.g. clang-format 10.

@ax3l
Copy link
Member

ax3l commented May 18, 2020

Sounds nice, go for it :)
Please feel free to push onto #32 or cherry-pick from it as needed.

@j-stephan
Copy link
Collaborator

j-stephan commented Sep 17, 2020

Carrying over the discussion from #19 to here.

clang-format appears to be relatively easy, I don't think we need to patch the LLVM source code for that.

However we will also require clang-tidy because clang-format doesn't handle stuff like camelCase or snake_case. I did a first test run on the alpaka codebase yesterday which immediately uncovered some bugs in clang-tidy. (Example: Constant parameters to functions/methods are not correctly detected and thus not fixed if they have the wrong casing.)

While working on the clang-tidy file I also noticed a huge load of additional options that (to me) appear immensely useful but are not really style-related. I suggest the following procedure:

  1. I will open a PR for the clang-format file along with a plain text file which includes the reasoning for the individual options.
  2. In parallel I will open a few bug reports on the clang-tidy tracker unless we want to cover the related issues by hand.
  3. I will open separate PRs for each of the clang-tidy options: casing, best practices, code modernization, ... along with text files that explain the reasoning.

Of course all PRs will be up for debate.

In the end we should have a nice style / best practices guide along with the means to actually enforce the rules. And it appears that CMake actually has builtin support for (at least) clang-tidy, so that is nice, too.

@sbastrakov
Copy link
Member

Nice plan of action @j-stephan !

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

No branches or pull requests

9 participants