Skip to content

Commit ac36e14

Browse files
committed
Merge branch 'mjs/eamxx/docs_style-guide' into next (PR #7400)
Put together a style guide that covers: Formatting - The clang-format-based autoformatting workflow - General description of the format standard and configuration - Summary of the workflow - How to install/configure/use clang-format on your development machine Style - General guidelines - Specific guidelines for: - Types - Functions/Methods - Variables - Templating
2 parents 5cd1db5 + 4694e8c commit ac36e14

File tree

15 files changed

+790
-43
lines changed

15 files changed

+790
-43
lines changed

components/eamxx/.clang-format

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ AlignConsecutiveMacros: true
77
AlignEscapedNewlines: true
88
AlignTrailingComments: true
99
---
10-
# # these are the defaults for the LLVM style, generated by
10+
# # you can obtain the defaults for the LLVM style by running
1111
# # clang-format --style=llvm --dump-config > [outfile]
1212
# # the option definitions are found here:
1313
# # https://clang.llvm.org/docs/ClangFormatStyleOptions.html
Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
# EAMxx Code Formatting Standards
2+
3+
To enforce consistent code format throughout EAMxx, we make use of an
4+
autoformatting workflow, carried out via Github Actions in the E3SM repository.
5+
6+
- The tool we employ is
7+
[`clang-format`](https://clang.llvm.org/docs/ClangFormat.html),
8+
and the version we have chosen is v14.[^v14]
9+
<!-- FIXME: Nail this down when figured out -->
10+
- The standard we maintain is largely identical to the
11+
[LLVM Coding Standards](https://llvm.org/docs/CodingStandards.html),
12+
and a list of the handful of customizations of this convention are
13+
enumerated in the configuration file [`$EAMXX_ROOT/.clang-format`](https://github.com/E3SM-Project/E3SM/blob/master/components/eamxx/.clang-format).
14+
- See this [How-to Guide](resources/clang-format_HOWTO.md)
15+
for additional details on how to configure `clang-format` on your chosen
16+
development machine.
17+
18+
## Automated Workflow Summary
19+
20+
- The `eamxx-format` workflow runs automatically and passes or fails based on
21+
adherence to our formatting standard.
22+
- The workflow is triggered by any Pull Request (PR) that modifies EAMxx code.
23+
- It is also triggered by other PR-related triggers, such as pushing
24+
changes, or converting from **draft** to **ready**.
25+
- All code modified by a PR must be formatted prior to **merging**.
26+
- It is not necessary for your code to be formatted upon ***opening*** a
27+
Pull Request, but feel free to `clang-format` as you develop if that is
28+
your preference.
29+
- The one situation for which opening a pre-formatted PR may not be
30+
preferred is if the file has never previously been `clang-format`-ed
31+
and requires a large number of changes.
32+
- I.e., touching the majority of lines in a file for format-only
33+
changes will make it difficult for a reviewer to determine which
34+
lines were changed for substantive reasons.
35+
- In this case, please refrain from formatting the code prior to
36+
opening the PR, and, instead, run `clang-format` once the PR is
37+
approved to be merged and make that the final commit.
38+
- As of now, the `eamxx-format` workflow only considers files that are edited
39+
by the Pull Request.
40+
- In addition to the pass/fail status, the workflow, provides the path to any
41+
files that caused the failure.
42+
- This information is found on the ***Summary*** page for any failed
43+
`eamxx-format` ***Job***.[^huh-where]
44+
45+
[^v14]: It turns out that this is important because there really are
46+
differences in behavior across versions.
47+
[^huh-where]: To get to this summary, select the ***Checks*** tab at the top of
48+
the PR page and select the `eamxx-format` workflow from the left sidebar.
49+
The summary is in the main pane of the page with the title
50+
**clang-format-linter summary**.[^also]
51+
[^also]: Note that this can also be accessed the long way around by following the
52+
***Actions*** link at the top of the E3SM repository page;
53+
select `eamxx-format` from the ***All workflows*** section of the ***Actions***
54+
sidebar; then choose the most recent run that is associated with your PR,
55+
which should be near the top of the list.
Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
# Functions and Methods
2+
3+
## Naming
4+
5+
- Please name functions (methods) to be ***descriptive*** and ***verb**-ish* so
6+
that the name describes what the function *does*.
7+
- For example, `install_flux_capacitor()` instead of `f_capacitor_method()`
8+
or `ifc()`.
9+
- To note, if your function cannot be named verb-ishly, that is probably a
10+
sign that there is a fundamental issue with your function.
11+
- In general, functions should use `snake_case` and not contain capitalization,
12+
unless capitalizing that quantity is a standard convention (e.g.,
13+
`find_Doc_Brown()`).
14+
15+
## Usage
16+
17+
- In situations where multiple class-member variables are passed as function
18+
arguments, favor passing the object, rather than the individual variables,
19+
for the sake of shorter lines and function signatures.
20+
- As a rule of thumb, developers should prefer passing arguments by reference,
21+
rather than by value, especially in cases for which the argument is ***large***
22+
or structurally ***complex***.
23+
- E.g., prefer `func(Type &x) { ... }` versus `func(Type x) { ... }`.
24+
- This holds true much of the time because passing by value creates a copy
25+
of the argument for use by the function, and this increases memory
26+
pressure and a resultant performance penalty.
27+
- To be more in-depth on the topic, we echo the guidance from the
28+
[***C++ Core Guidelines***](https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#S-introduction)
29+
and would encourage the curious to read more deeply on the topic.
30+
31+
> - For "in" parameters, pass cheaply-copied types by value and others by
32+
> reference to `const`.
33+
> - For "in-out" parameters, pass by reference to non-`const`.
34+
> - For "out" parameters, prefer return values to output parameters.
35+
36+
- The authors go on to explain that "cheap to copy" includes variables
37+
holding 2 or 3 words--for example a double, pointer, or reference.
38+
- To illustrate, the below function adheres to the provided guidance.
39+
40+
```c++
41+
SmallStruct func(LargeNComplex &notSmall, double forty_two,
42+
const int *laser, const BigStruct &lotsaData) {
43+
44+
// SmallStruct object is only assigned-to and then returned, so it is
45+
// defined as the return type
46+
SmallStruct ans;
47+
// forty_two, a scalar double, is considered small and so passed by value
48+
ans.val = forty_two;
49+
// lotsaData, a BigStruct object, is only accessed and so is
50+
// passed by const reference
51+
ans.other_val = lotsaData.small_piece;
52+
53+
// we pass laser by value and as const because pointers are cheap to copy,
54+
// and the value is not changed
55+
for (int i = 0; i < forty_two; ++i) {
56+
ans.val_vector.push_back(laser[i] + notSmall.epsilon[i]);
57+
}
58+
59+
// notSmall is large and also modified in the function, so we pass by
60+
// reference
61+
// it is also accessed, above, so it must be an argument and not the
62+
// return value
63+
notSmall.ans_struct = ans;
64+
return ans;
65+
}
66+
```
Lines changed: 208 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,208 @@
1+
# How-to Guide: `clang-format`
2+
3+
This guide is for developers who wish to apply `clang-format` on their chosen
4+
development machine, whether that be their personal machine or a multi-user
5+
cluster.
6+
In this guide, we will describe how to configure/run `clang-format` on EAMxx
7+
code and also how to install it if necessary.
8+
9+
## Configure and Run `clang-format`
10+
11+
Running `clang-format` according to the defined EAMxx standard ***can*** be
12+
done using only command line arguments; however, the command is quite long.
13+
The easier route is to reference the configuration file
14+
(`$EAMXX_ROOT/.clang-format`).
15+
In this case the command is
16+
17+
```bash {.copy}
18+
clang-format [-i] --style="file:${EAMXX_ROOT}/.clang-format" <path/to/c++/source/file(s)>
19+
```
20+
21+
where the `-i` (`--in-place`) argument controls whether the formatting edits
22+
are conducted and change the source file(s) ("in place") or the required edits
23+
are printed to `stdout` (flag omitted).
24+
Also, note that the `--style` flag can also fully define the options without
25+
the configuration file as follows, but this is not recommended as the
26+
configuration could change before this guide is updated to reflect as such.
27+
28+
<!-- markdownlint-disable MD046 MD013 -->
29+
??? Danger "Terminal One-(long-)liner"
30+
31+
```bash {.copy}
32+
clang-format [-i] --style="{BasedOnStyle: llvm, ColumnLimit: 100, AlignConsecutiveAssignments: true, AlignConsecutiveBitFields: true, AlignConsecutiveMacros: true, AlignEscapedNewlines: true, AlignTrailingComments: true}" <path/to/c++/source/file(s)>
33+
```
34+
<!-- markdownlint-enable MD046 MD013 -->
35+
36+
## Installing `clang-format`
37+
38+
On a personal machine, which we will consider to be one for which you have
39+
`sudo` privileges, installation can be conducted via package manager or by
40+
building `llvm v14` from scratch.
41+
If you are a non-admin user of a multi-user cluster, HPC platform, etc., then
42+
it is likely to be an ***easy*** process, though potentially not immediate.
43+
44+
<!-- markdownlint-disable MD046 -->
45+
??? Note "If you require or already have multiple versions of `clang-format` installed"
46+
47+
Note that, depending on your requirements, this could be placed in your shell
48+
config file (`.bashrc`, `.zshrc`), or if only needed for a shell session, it
49+
can be done directly from the command line.
50+
51+
The other option is to add a versioned symbolic link to your `PATH`.
52+
This is sometimes included in the package's `bin/` directory by default and,
53+
if not, can be added there after of placed somewhere that is already on your
54+
`PATH`.
55+
56+
```c++
57+
$ cd /opt/homebrew/opt/llvm@14/bin
58+
$ ls clang-format*
59+
clang-format
60+
// no versioned binary so we create symlink
61+
$ ln -s ./clang-format ./clang-format-14
62+
$ which clang-format
63+
/opt/homebrew/opt/llvm@14/bin/clang-format-14
64+
```
65+
66+
**OR**
67+
68+
```c++
69+
$ cd /opt/homebrew/opt/llvm@14/bin
70+
$ ls clang-format*
71+
clang-format
72+
// no versioned binary so we create symlink in a directory already on PATH
73+
$ echo $PATH
74+
/usr/bin:/usr/local/bin
75+
$ ln -s ./clang-format /usr/local/bin/clang-format-14
76+
$ which clang-format
77+
/usr/local/bin/clang-format-14
78+
```
79+
<!-- markdownlint-enable MD046 -->
80+
81+
=== "Personal Machine"
82+
83+
<!-- markdownlint-disable MD046 -->
84+
=== "Mac Users (Homebrew Package Manager)"
85+
86+
For Mac users, the [Homebrew](https://brew.sh)
87+
package manager (`brew`) is the quickest and most straightforward way
88+
to get `clang-format` installed.
89+
Since `clang-format` v14 is not available to install directly from
90+
Homebrew, we install the entire LLVM package at version 14, and this
91+
is as simple as
92+
93+
```bash {.copy}
94+
brew install llvm@14
95+
```
96+
97+
It it likely that Homebrew will issue a message about not linking the
98+
`clang`-related tools by default, so next we add the binary to our `PATH`.
99+
100+
```bash {.copy}
101+
$ export PATH="/opt/homebrew/opt/llvm@14/bin/clang-format:$PATH"
102+
# Note: this is the default location for a recent Mac running Apple silicon.
103+
# It may be different on another system.
104+
# You can confirm where yours is installed via 'brew info llvm@14'
105+
$ which clang-format
106+
/opt/homebrew/opt/llvm@14/bin/clang-format
107+
```
108+
109+
Note also, that if your system has multiple version of `clang-format` installed,
110+
it may be preferable to instead set a versioned alias to `clang-format`
111+
(`clang-format-14`) as in
112+
113+
```c++
114+
// create a shell-alias
115+
alias clang-format-14="/opt/homebrew/opt/llvm@14/bin/clang-format"
116+
```
117+
118+
=== "Linux Users (Package Manager)"
119+
120+
Given the many flavors of Linux, it is difficult to generalize, but there
121+
is a high probability the proper version of `clang-format` or `llvm` is
122+
provided by the built-in package manager.
123+
The commands will differ based on your Linux distribution, but using the
124+
Debian/Ubuntu `apt` syntax, it could be accomplished via something like
125+
126+
```bash {.copy}
127+
$ apt search clang-format
128+
[...]
129+
clang-format-14/...
130+
$ apt install clang-format-14
131+
```
132+
133+
=== "Build from Source"
134+
135+
If you do not succeed in the above, `clang-format` can also be fully built
136+
from the [LLVM Compiler Infrastructure](https://github.com/llvm/llvm-project).
137+
It will begin with something like
138+
139+
```bash {.copy}
140+
git clone git@github.com:llvm/llvm-project.git
141+
git checkout llvmorg-14.0.6 # version tag
142+
```
143+
144+
Also, if you only need `clang-format` and not any of the other tooling,
145+
it will build faster/smaller if you use the CMake flag
146+
`-DLLVM_ENABLE_PROJECTS="clang"` to only build `clang` and it's friends,
147+
rather than all of `llvm`.
148+
And finally, the README for [LLVM version 14.0.6](https://github.com/llvm/llvm-project/tree/llvmorg-14.0.6)
149+
is far more comprehensive that the one for the latest version, and it contains
150+
instructions specific to that build.
151+
152+
=== "Multi-user System"
153+
154+
In many cases `llvm`, `clang`, or `clang-format` will be available as a module,
155+
though whether version 14 is an option could be less likely.
156+
In the optimistic case, it could be as simple as (using Lmod syntax)
157+
158+
```bash {.copy}
159+
$ module avail llvm # [clang, clang-format]
160+
[... list ]
161+
$ module load llvm/14.0.6
162+
```
163+
164+
If it is not available, you will probably need to reach out to a system
165+
administrator to get an official version installed.[^but-conda]
166+
<!-- markdownlint-enable MD046 -->
167+
168+
---
169+
170+
<!-- markdownlint-disable MD046 -->
171+
??? Tip "Unnecessary but Convenient Workflow Customization (`direnv`)"
172+
173+
If you'd like to add a layer of automation/complexity to ensure you only use
174+
`clang-format v14` on `EAMxx` and/or want to use a newer version on the rest
175+
of your system, there is a very handy terminal tool called
176+
[direnv](https://direnv.net/)
177+
(`brew install direnv`) that allows you to automatically load and unload
178+
environment variables based on `$PWD` using a `.envrc` file.
179+
As an example, here's my `.envrc` that adds `clang-format v14` to the path
180+
when I'm working on `EAMxx`.
181+
182+
```bash {.copy}
183+
PATH_add /opt/homebrew/opt/llvm@14/bin/clang-format
184+
185+
# also, since I often forget to load a python environment that's required for
186+
# running ctest, this creates or loads a python 3 virtual environment with numpy
187+
layout python3
188+
pip install --upgrade pip
189+
# the upgrade isn't strictly necessary but trades a little extra setup on
190+
# the front end to avoid pip endlessly reminding you to update
191+
pip install numpy
192+
```
193+
194+
This file can be placed in the top-level `EAMxx` directory, and running
195+
`direnv allow` enables the functionality.
196+
Namely, executing `cd EAMxx` loads the defined environment that stays loaded
197+
in any subdirectories, and resets the standard environment when exiting to a
198+
directory above/outside of `EAMxx`.
199+
200+
For the `conda` fans, this tool can also be used to auto-load a
201+
pre-configured `conda` environment since the `.envrc` is essentially a bash
202+
script with a few bells and whistles tacked on.
203+
<!-- markdownlint-enable MD046 -->
204+
205+
<!-- markdownlint-disable MD053 -->
206+
[^but-conda]: There are rumors of using `conda` creatively to do a user-install,
207+
but that is not an option we support or suggest.
208+
<!-- markdownlint-enable MD053 -->
Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
# EAMxx Code Style Standards
2+
3+
EAMxx does not currently impose strict styling standards, other than those of
4+
the autoformatter.
5+
However, if we can follow consistent style and usage guidelines, we can make
6+
EAMxx developers' lives easier and turnaround times quicker.
7+
In the age of modern text editors with autocomplete and running our code on
8+
machines with enormous storage capacity, there is no real need to save screen
9+
or disk space with minimal naming strategies or other meaning-obscuring style
10+
choices.
11+
So, please adhere to the spirit of these guidelines, and your fellow developers
12+
will thank you for it!
13+
14+
## General Guidelines
15+
16+
- Please give ***descriptive***, rather than ***terse***, names to your
17+
variables, functions, classes, etc.
18+
- Avoid idiosyncratic naming or styling conventions.
19+
- If the utility of a style decision would not be immediately apparent to
20+
a reasonable developer, please avoid its usage.
21+
- In the hierarchy of coding concerns, correctness and speed take the top
22+
tiers, but below that level, please favor readability and clarity over
23+
space savings, line-breaking, heavy use of arcane language features, etc.
24+
- That said, if an arcane language feature ***is*** the correct tool to
25+
be used, please do so.
26+
- And perhaps add a comment to aid the non-Jedi-Master developers who
27+
read the code next. :slightly_smiling_face:
28+
- With regard to comments, the general wisdom is that the correct amount of
29+
comments is the amount required to make the code clear and easy to understand.
30+
- To quote
31+
[Jeff Atwood](https://blog.codinghorror.com/code-tells-you-how-comments-tell-you-why/)
32+
from his essay about code comments:
33+
> Only at the point where the code *cannot* be made easier to understand
34+
should you begin to add comments.
35+
- However, since that does not offer much in the way of prescriptive
36+
guidance, here are some guidelines gathered from around the internet.
37+
- A general rule of thumb, though not always true, is that comments
38+
should explain ***why*** something is being done in the code,
39+
rather than ***what*** the code is doing.[^butwhattabout]
40+
- Comments should dispel confusion and not cause it.
41+
- Comments should not duplicate the code.[^dupe]
42+
- Provide links or references when using code from elsewhere or
43+
whenever it is otherwise appropriate.
44+
- A bad comment is worse than no comment.
45+
- Add comments to explain non-standard usage or unidiomatic code.
46+
47+
[^butwhattabout]: An obvious exception to this is explaining complex or opaque
48+
parts of the code that cannot be made simpler--for instance, a clever arithmetic
49+
trick in a complicated interpolation scheme.
50+
[^dupe]: For example, this type of comment does not add any information and
51+
is unnecessary.
52+
53+
```c++
54+
// perform initialization
55+
this->initialize();
56+
```

0 commit comments

Comments
 (0)