Project

General

Profile

Coding Standards » History » Version 31

Brett Smith, 04/12/2023 03:09 PM

1 1 Tom Clegg
h1. Coding Standards
2
3 3 Tom Clegg
The rules are always up for debate. However, when debate is needed, it should happen outside the source tree. In other words, if the rules are wrong, first debate the rules in IRC etc., then fix the rules, then follow the new rules.
4 1 Tom Clegg
5 2 Tom Clegg
{{toc}}
6 1 Tom Clegg
7 2 Tom Clegg
h2. Git commits
8
9 1 Tom Clegg
Make sure your name and email address are correct.
10
11
* Use @git config --global user.email foo@example.com@ et al.
12
* It's a little unfortunate to have commits with author @foo@myworkstation.local@ but not bad enough to rewrite history, so fix this before you push!
13
14 19 Tom Clegg
Refer to a story number in the first (summary) line of each commit comment. This first line should be <80 chars long, and should be followed by a blank line.
15 9 Tom Clegg
16
* @1234: Remove useless button.@
17
18
*When merging/committing to master,* refer to the story number in a way Redmine will notice. Redmine will list these commits/merges on the story page itself.
19
20 1 Tom Clegg
* @closes #1234@, or
21 19 Tom Clegg
* @refs #1234@, or
22
* @no issue #@ if no Redmine issue is especially relevant.
23 9 Tom Clegg
24 1 Tom Clegg
Use descriptive commit comments.
25
26
* Describe the delta between the old and new tree. If possible, describe the delta in *behavior* rather than the source code itself.
27 9 Tom Clegg
* Good: "1234: Support use of spaces in filenames."
28
* Good: "1234: Fix crash when user_id is nil."
29 1 Tom Clegg
* Less good: "Add some controller methods." (What do they do?)
30
* Less good: "More progress on UI branch." (What is different?)
31
* Less good: "Incorporate Tom's suggestions." (Who cares whose suggestions -- what changed?)
32
33
If further background or explanation is needed, separate it from the summary with a blank line.
34
35
* Example: "Users found it confusing that the boxes had different colors even though they represented the same kinds of things."
36
37 18 Tom Clegg
*Every commit* (even merge commits) must have a DCO sign-off. See [[Developer Certificate Of Origin]].
38 1 Tom Clegg
39
* Example: <code>Arvados-DCO-1.1-Signed-off-by: Joe Smith <joe.smith@example.com></code>
40 19 Tom Clegg
41
Full examples:
42
43
<pre>
44
commit 9c6540b9d42adc4a397a28be1ac23f357ba14ab5
45
Author: Tom Clegg <tom@curoverse.com>
46
Date:   Mon Aug 7 09:58:04 2017 -0400
47
48
    12027: Recognize a new "node failed" error message.
49
    
50
    "srun: error: Cannot communicate with node 0.  Aborting job."
51
    
52
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom@curoverse.com>
53
</pre>
54
55
<pre>
56
commit 0b4800608e6394d66deec9cecea610c5fbbd75ad
57
Merge: 6f2ce94 3a356c4
58
Author: Tom Clegg <tom@curoverse.com>
59
Date:   Thu Aug 17 13:16:36 2017 -0400
60
61
    Merge branch '12081-crunch-job-retry'
62
    
63
    refs #12080
64
    refs #12081
65
    refs #12108
66
    
67
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom@curoverse.com>
68
</pre>
69
70 21 Ward Vandewege
h2. Copyright headers
71
72 23 Ward Vandewege
Each Arvados component is released either under the AGPL 3.0 license or the Apache 2.0 license. Documentation is licensed under CC-BY-SA-3.0. See the [[Arvados Licenses FAQ]] for the rationale behind this system.
73 21 Ward Vandewege
74 22 Ward Vandewege
Every file must start with a copyright header that follows this format:
75 21 Ward Vandewege
76
Code under the "AGPLv3 license":http://www.gnu.org/licenses/agpl-3.0.en.html (this example uses Go formatting):
77
78
<pre>
79
// Copyright (C) The Arvados Authors. All rights reserved.
80
//
81
// SPDX-License-Identifier: AGPL-3.0
82
</pre>
83
84
Code under the "Apache 2.0 license":http://www.apache.org/licenses/LICENSE-2.0 (this example uses Python formatting):
85
86
<pre>
87
# Copyright (C) The Arvados Authors. All rights reserved.
88
#
89
# SPDX-License-Identifier: Apache-2.0
90
</pre>
91
92
Documentation under the "Creative Commons Attribution-Share Alike 3.0 United States license":https://creativecommons.org/licenses/by-sa/3.0/us/ (this example uses textile formatting):
93
94
<pre>
95
###. Copyright (C) The Arvados Authors. All rights reserved.
96
....
97
.... SPDX-License-Identifier: CC-BY-SA-3.0
98
</pre>
99
100 1 Tom Clegg
When adding a new file to a component, use the same license as the other files of the component.
101
102 22 Ward Vandewege
When adding a new component, choose either the AGPL or Apache license. Generally speaking, the Apache license is only used for components where integrations in proprietary code must be possible (e.g. our SDKs), though this is not a hard rule. When uncertain which license to choose for a new component, ask on the IRC channel or mailing list.
103 21 Ward Vandewege
104 22 Ward Vandewege
When adding a file in a format that does not support the addition of a copyright header (e.g. in a binary format like an image), add the path to the .licenseignore file in the root of the source tree. This should be done sparingly, and must be discussed explicitly as part of code review. The file must be available under a license that is compatible with the rest of the Arvados code base.
105 21 Ward Vandewege
106 22 Ward Vandewege
When adding a file that originates from an external source under a different license, add the appropriate SPDX line for that license. This is exceptional, and must be discussed explicitly as part of code review. Not every license is compatible with the rest of the Arvados code base.
107 16 Tom Clegg
108 28 Ward Vandewege
There is a helper script at https://github.com/arvados/arvados/blob/master/build/check-copyright-notices that can be used to check - and optionally, fix - the copyright headers in the Arvados source tree.
109 24 Ward Vandewege
110 29 Ward Vandewege
The actual git hook that enforces the copyright headers lives at https://github.com/arvados/arvados-dev/blob/master/git/hooks/check-copyright-headers.rb
111 25 Ward Vandewege
112 13 Tom Clegg
h2. Source code formatting
113 1 Tom Clegg
114 13 Tom Clegg
(Unless otherwise specified by style guide...)
115
116 10 Tom Clegg
No TAB characters in source files. "Except go programs.":https://golang.org/cmd/gofmt/
117 1 Tom Clegg
118 6 Tom Clegg
* Emacs: add to @~/.emacs@ &rarr; @(setq-default indent-tabs-mode nil)@
119
* Vim: add to @~/.vimrc@ &rarr; @:set expandtab@
120 8 Tom Clegg
* See [[Coding Standards#Git setup|Git setup]] below
121 4 Ward Vandewege
122 6 Tom Clegg
No inline comments: @this = !desired; # we don't want to do it.@
123 4 Ward Vandewege
124 6 Tom Clegg
No long (>80 column) lines, except in rare cases when the alternative is really clunky.
125
126 4 Ward Vandewege
No whitespace at the end of lines. Make git-diff show you:
127 5 Ward Vandewege
128
  git config color.diff.whitespace "red reverse"
129 6 Tom Clegg
git diff --check
130 1 Tom Clegg
131 13 Tom Clegg
h2. What to include
132
133 1 Tom Clegg
No commented-out blocks of code that have been replaced or obsoleted.
134
135
* It is in the git history if we want it back.
136
* If its absence would confuse someone reading the new code (despite never having read the old code), explain its absence in an English comment. If the old code is really still needed to support the English explanation, then go ahead -- now we know why it's there.
137
138
No commented-out debug statements.
139
140
* If the debug statements are likely to be needed in the future, use a logging facility that can be enabled at run time. @logger.debug "foo"@
141
142 13 Tom Clegg
h2. Style mismatch
143
144 1 Tom Clegg
Adopt indentation style of surrounding lines or (when starting a new file) the nearest existing source code in this tree/language.
145
146
If you fix up existing indentation/formatting, do that in a separate commit.
147
* If you bundle formatting changes with functional changes, it makes functional changes hard to find in the diff.
148
149 13 Tom Clegg
h2. Go
150
151
gofmt, golint, etc., and https://github.com/golang/go/wiki/CodeReviewComments
152
153
h2. Ruby
154
155
https://github.com/bbatsov/ruby-style-guide
156
157 1 Tom Clegg
h2. Python
158 13 Tom Clegg
159 30 Brett Smith
h3. Python code
160 12 Tom Clegg
161 30 Brett Smith
For code, follow "PEP 8":https://peps.python.org/pep-0008/.
162 1 Tom Clegg
163 30 Brett Smith
When you add functions, methods, or attributes that SDK users should not use, their name should start with a leading underscore. This is a common convention to signal that an interface is not intended to be public. Anything named this way will be excluded from our SDK web documentation by default.
164
165
h3. Python docstrings
166
167
Public classes, methods, and functions should all have docstrings. The content of the docstring should follow "PEP 257":https://peps.python.org/pep-0257/.
168
169
Format docstrings with Markdown and follow these style rules:
170
171
* Document function argument lists as a definition list after the high-level description. The defined term should be the name of the argument and, whenever practical, its type as it would be written in the function signature. The definition should describe the argument's purpose.
172
* Document class constructor arguments in the class docstring. The @__init__@ method is not rendered in the web documentation by default.
173
* When something is deprecated, write a @!!! deprecated@ admonition immediately after the first line. Its text should explain that the thing is deprecated, and suggest what to use instead. For example:<pre>def add(a, b):
174
    """Add two things.
175
176
    !!! deprecated
177
        This function is deprecated. Use the `+` operator instead.
178
179
180
    """</pre>
181
* Mark up all identifiers with backticks. When the identifier exists in the current module, use the short name. Otherwise, use the fully-qualified name. Our web documentation will automatically link these identifiers to their corresponding documentation.
182
* Mark up links using Markdown's footnote style. For example:<pre>"""Python docstring following [PEP 257][pep257].
183
184
[pep257]: https://peps.python.org/pep-0257/
185
"""</pre>This looks best in plaintext. A descriptive identifier is nice if you can keep it short, but if that's challenging, plain ordinals are fine too.
186
* Mark up headers (e.g., in a module docstring) using underline style. For example:<pre>"""Generic utility module
187
188
Filesystem functions
189
--------------------
190
191
192
193
Regular expressions
194
-------------------
195
196
197
"""</pre>This looks best in plaintext.
198
199
The goal of these style rules is to provide a readable, consistent appearance whether people read the documentation in plain text (e.g., using @pydoc@) or their browser (as rendered by @pdoc@).
200
201 12 Tom Clegg
202 11 Brett Smith
h2. JavaScript
203
204 20 Tom Clegg
Follow the Airbnb Javascript coding style guide unless otherwise stated:
205 14 Tom Morris
https://github.com/airbnb/javascript
206 20 Tom Clegg
207
We already have 4-space indents everywhere, though, so do that.
208
209 14 Tom Morris
210 7 Tom Clegg
h2. Git setup
211 6 Tom Clegg
212 7 Tom Clegg
Configure git to prevent you from committing whitespace errors.
213 1 Tom Clegg
214 6 Tom Clegg
<pre>
215 7 Tom Clegg
git config --global core.whitespace tab-in-indent,trailing-space
216 1 Tom Clegg
git config --global apply.whitespace error
217 17 Tom Clegg
</pre>
218
219
Add a DCO sign-off to the default commit message.
220
221
<pre>
222
cd .../arvados
223
printf '\n\nArvados-DCO-1.1-Signed-off-by: %s <%s>\n' "$(git config user.name)" "$(git config user.email)" >~/.arvados-dco.txt
224
git config commit.template ~/.arvados-dco.txt
225
</pre>
226
227
Add a DCO sign-off and "refs #xxxx" comment (referencing the issue# in the name of the branch being merged) to the default merge commit message.
228
229
<pre>
230
cd .../arvados
231 26 Eric Biagiotti
cat >.git/hooks/prepare-commit-msg <<'EOF'
232 17 Tom Clegg
#!/bin/sh
233
234
case "$2,$3" in
235
    merge,)
236
        br=$(head -n1 ${1})
237
        n=$(echo "${br}" | egrep -o '[0-9]+')
238
        exec >${1}
239
        echo "${br}"
240
        echo
241
        echo "refs #${n}"
242
        echo
243 27 Eric Biagiotti
        echo "Arvados-DCO-1.1-Signed-off-by: $(git config user.name) <$(git config user.email)>"
244 17 Tom Clegg
        ;;
245
    *)
246
        ;;
247
esac
248
EOF
249 26 Eric Biagiotti
chmod +x .git/hooks/prepare-commit-msg
250 6 Tom Clegg
</pre>
251 31 Brett Smith
252
h2. GUI (Workbench 2)
253
254
h3. Font Sizes
255
256
* Minimum 12pt (16px) 
257
* Minimum 9 pt (12px) for things like by copyright, footer
258
259
This should be able to be-resized up to 200% without loss of content or functionality.
260
261
h3. Color
262
263
* Text and images of text have a color contrast ratio of at least 4.5:1 You can use "this contrast tool":https://snook.ca/technical/colour_contrast/colour.html#fg=1F7EA1,bg=FFFFFF to check.
264
* Non-text icon, controls, etc - 3:1 must have a color contrast ratio of 3:1.
265
* Avoid hard-coding colors. Use theme colors. If a new color is needed, add it to the theme.  
266
* Used defined grays when possible using RGB value and changing the a value to indicate different meanings (i.e. Active icons have an opacity of 87%, Inactive icons have an opacity of 60%, Disabled icons have an opacity of 38%)
267
268
h3. Icons
269
270
h4. General
271
272
* Interaction target size of at least 44 x 44 pixels
273
* Label should be on right, icon on left for maximum readability
274
* Use minimum 3:1 color contrast (see Color above)
275
* User appropriate concise alt text for people using screen readers 
276
277
h4. Menu/Navigation 
278
279
* No navigation should only supported via breadcrumbs
280
* If less than 5 menu options, consider visible navigation options
281
* If more than 5 menu options, consider a combination navigation where some options are visible and some are hidden
282
* Use the following menu consistently:	
283
** Hamburger (three bars stacked vertically): Used to indicate navigation bar/menu that toggles between being collapsed behind the button or displayed on the screen, often used for global/site-wide/whole application navigation
284
** Döner (three bars that narrow vertically):  Indicates a group filtering menu
285
** Bento (3×3 grid of squares):  Indicates a menu presenting a grid of options (not currently applicable to WB)
286
** Kebab (three dots stacked vertically): Indicates a smaller inline-menu or an overflow/combination menu
287
** Meatballs (three dots stacked horizontally):  Used to indicate a smaller inline-menu.  Often used to indicate action on a related item (i.e. item next to the meatball), good for repeated use in tables, or horizontal elements
288
* If component is an accordion window,  use caret(‸)
289
290
Preferred Icon Repositories:
291
* https://v4.mui.com/components/material-icons/
292
* https://materialdesignicons.com/
293
* https://fontawesome.com/v5/search
294
295
h3. Buttons
296
297
* Label button with action for usability/to reduce ambiguity (avoid generic button labels for actions)
298
* Buttons vs Links
299
** Buttons should cause change in current context
300
** Links should navigate to a different content or a new resource (e.g. different page)
301
* If text on button - color contract should be 4.5 :1 between button and text
302
* Button color and background color contrast should be 3:1
303
304
h3. Arvados Specific Components
305
306
Use chips for displaying tokenized values/arrays
307
308
h3. References
309
310
"WCAG2.1":https://www.w3.org/WAI/WCAG21/Understanding/
311
312
"Sarah’s talk for references":https://docs.google.com/presentation/d/1HNrhvK7zVZ7jgH3ELbX7KB97SdXCZXrvov_I4Oe1l2c/edit?usp=sharing