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

Implement vt's style guidelines #105

Closed
cwschilly opened this issue Aug 9, 2024 · 5 comments · Fixed by #109
Closed

Implement vt's style guidelines #105

cwschilly opened this issue Aug 9, 2024 · 5 comments · Fixed by #109
Assignees

Comments

@cwschilly
Copy link
Contributor

cwschilly commented Aug 9, 2024

As mentioned in this comment by @nlslatt

@tlamonthezie tlamonthezie self-assigned this Aug 27, 2024
@tlamonthezie
Copy link
Contributor

Hello @cwschilly it seems that the script generating the header in .h and .cc files has already been integrated in #85
Is there anything else related to guidelines in the current issue ?

@cwschilly
Copy link
Contributor Author

Hi @tlamonthezie

Yes the license issue should be resolved. The remaining work on this issue is to go through the vt-tv code and make sure that all style guidelines from vt are being followed.

For reference:

@tlamonthezie
Copy link
Contributor

tlamonthezie commented Aug 28, 2024

Checks for src and tests directories

  1. clang-format
  • Add .clang-format configuration file from vt (edit: std must be c++17)
  • Format files using clang-format
  1. vt's style guidelines:
  • Do not use "typedef", always use "using"
  • Prefer "struct" over "class
  • Wrap typename args when they get long, start ">" on next line
  • Use an enum struct when possible
  • A new line should be after the template
  • Makes it easier to identify the full namespace path (nested namespace in 1 line as possible). Edit: Because std is C++17, prefer use namespace A::B {} over namespace A { namespace B { } } and make it consistent.
  1. vt's Doxygen style guidelines
  • Do some checks about doxygen styles (src directory)

@tlamonthezie
Copy link
Contributor

tlamonthezie commented Aug 28, 2024

@cwschilly @lifflander I noticed that in the header the licence contains the line "// Copyright 2019-2024 National Technology & Engineering Solutions of Sandia, LLC" which is 81 characters length. This is not compliant with the 80 char max per line guideline.
Should we break that line before "LLC" ?
Note: this is also the case in vt

@cwschilly
Copy link
Contributor Author

@tlamonthezie We probably want to stick with whatever vt is using--even if it goes past 80 char

tlamonthezie added a commit that referenced this issue Aug 28, 2024
tlamonthezie added a commit that referenced this issue Sep 4, 2024
tlamonthezie added a commit that referenced this issue Sep 4, 2024
tlamonthezie added a commit that referenced this issue Sep 4, 2024
tlamonthezie added a commit that referenced this issue Sep 4, 2024
tlamonthezie added a commit that referenced this issue Sep 4, 2024
tlamonthezie added a commit that referenced this issue Sep 4, 2024
tlamonthezie added a commit that referenced this issue Sep 4, 2024
tlamonthezie added a commit that referenced this issue Sep 4, 2024
tlamonthezie added a commit that referenced this issue Sep 4, 2024
tlamonthezie added a commit that referenced this issue Sep 4, 2024
tlamonthezie added a commit that referenced this issue Sep 4, 2024
tlamonthezie added a commit that referenced this issue Sep 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants