Coding Standards » History » Version 21

Ward Vandewege, 05/22/2018 08:20 PM

1 1 Tom Clegg
h1. Coding Standards
2 1 Tom Clegg
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 2 Tom Clegg
9 1 Tom Clegg
Make sure your name and email address are correct.
10 1 Tom Clegg
11 1 Tom Clegg
* Use @git config --global user.email foo@example.com@ et al.
12 1 Tom Clegg
* 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 1 Tom Clegg
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 9 Tom Clegg
* @1234: Remove useless button.@
17 9 Tom Clegg
18 9 Tom Clegg
*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 9 Tom Clegg
20 1 Tom Clegg
* @closes #1234@, or
21 19 Tom Clegg
* @refs #1234@, or
22 19 Tom Clegg
* @no issue #@ if no Redmine issue is especially relevant.
23 9 Tom Clegg
24 1 Tom Clegg
Use descriptive commit comments.
25 1 Tom Clegg
26 1 Tom Clegg
* 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 9 Tom Clegg
* Good: "1234: Fix crash when user_id is nil."
29 1 Tom Clegg
* Less good: "Add some controller methods." (What do they do?)
30 1 Tom Clegg
* Less good: "More progress on UI branch." (What is different?)
31 1 Tom Clegg
* Less good: "Incorporate Tom's suggestions." (Who cares whose suggestions -- what changed?)
32 1 Tom Clegg
33 1 Tom Clegg
If further background or explanation is needed, separate it from the summary with a blank line.
34 1 Tom Clegg
35 1 Tom Clegg
* Example: "Users found it confusing that the boxes had different colors even though they represented the same kinds of things."
36 1 Tom Clegg
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 1 Tom Clegg
* Example: <code>Arvados-DCO-1.1-Signed-off-by: Joe Smith <joe.smith@example.com></code>
40 19 Tom Clegg
41 19 Tom Clegg
Full examples:
42 19 Tom Clegg
43 19 Tom Clegg
<pre>
44 19 Tom Clegg
commit 9c6540b9d42adc4a397a28be1ac23f357ba14ab5
45 19 Tom Clegg
Author: Tom Clegg <tom@curoverse.com>
46 19 Tom Clegg
Date:   Mon Aug 7 09:58:04 2017 -0400
47 19 Tom Clegg
48 19 Tom Clegg
    12027: Recognize a new "node failed" error message.
49 19 Tom Clegg
    
50 19 Tom Clegg
    "srun: error: Cannot communicate with node 0.  Aborting job."
51 19 Tom Clegg
    
52 19 Tom Clegg
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom@curoverse.com>
53 19 Tom Clegg
</pre>
54 19 Tom Clegg
55 19 Tom Clegg
<pre>
56 19 Tom Clegg
commit 0b4800608e6394d66deec9cecea610c5fbbd75ad
57 19 Tom Clegg
Merge: 6f2ce94 3a356c4
58 19 Tom Clegg
Author: Tom Clegg <tom@curoverse.com>
59 19 Tom Clegg
Date:   Thu Aug 17 13:16:36 2017 -0400
60 19 Tom Clegg
61 19 Tom Clegg
    Merge branch '12081-crunch-job-retry'
62 19 Tom Clegg
    
63 19 Tom Clegg
    refs #12080
64 19 Tom Clegg
    refs #12081
65 19 Tom Clegg
    refs #12108
66 19 Tom Clegg
    
67 19 Tom Clegg
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom@curoverse.com>
68 19 Tom Clegg
</pre>
69 19 Tom Clegg
70 21 Ward Vandewege
h2. Copyright headers
71 21 Ward Vandewege
72 21 Ward Vandewege
Each 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 21 Ward Vandewege
Every file must contain a copyright header that follows this format:
75 21 Ward Vandewege
76 21 Ward Vandewege
Code under the "AGPLv3 license":http://www.gnu.org/licenses/agpl-3.0.en.html (this example uses Go formatting):
77 21 Ward Vandewege
78 21 Ward Vandewege
<pre>
79 21 Ward Vandewege
// Copyright (C) The Arvados Authors. All rights reserved.
80 21 Ward Vandewege
//
81 21 Ward Vandewege
// SPDX-License-Identifier: AGPL-3.0
82 21 Ward Vandewege
</pre>
83 21 Ward Vandewege
84 21 Ward Vandewege
Code under the "Apache 2.0 license":http://www.apache.org/licenses/LICENSE-2.0 (this example uses Python formatting):
85 21 Ward Vandewege
86 21 Ward Vandewege
<pre>
87 21 Ward Vandewege
# Copyright (C) The Arvados Authors. All rights reserved.
88 21 Ward Vandewege
#
89 21 Ward Vandewege
# SPDX-License-Identifier: Apache-2.0
90 21 Ward Vandewege
</pre>
91 21 Ward Vandewege
92 21 Ward Vandewege
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 21 Ward Vandewege
94 21 Ward Vandewege
<pre>
95 21 Ward Vandewege
###. Copyright (C) The Arvados Authors. All rights reserved.
96 21 Ward Vandewege
....
97 21 Ward Vandewege
.... SPDX-License-Identifier: CC-BY-SA-3.0
98 21 Ward Vandewege
</pre>
99 21 Ward Vandewege
100 21 Ward Vandewege
101 21 Ward Vandewege
When adding a new file to a component, use the same license as the other files of the component.
102 21 Ward Vandewege
103 21 Ward Vandewege
When adding a new component, choose either the AGPL or Apache license. 
104 21 Ward Vandewege
105 21 Ward Vandewege
Generally speaking, we only use Apache for components where integrations in proprietary code should be possible (e.g. our SDKs), though this is not a hard rule.
106 21 Ward Vandewege
107 21 Ward Vandewege
When uncertain which license to choose for a new component, ask on the IRC channel or mailing list.
108 16 Tom Clegg
109 13 Tom Clegg
h2. Source code formatting
110 1 Tom Clegg
111 13 Tom Clegg
(Unless otherwise specified by style guide...)
112 13 Tom Clegg
113 10 Tom Clegg
No TAB characters in source files. "Except go programs.":https://golang.org/cmd/gofmt/
114 1 Tom Clegg
115 6 Tom Clegg
* Emacs: add to @~/.emacs@ &rarr; @(setq-default indent-tabs-mode nil)@
116 6 Tom Clegg
* Vim: add to @~/.vimrc@ &rarr; @:set expandtab@
117 8 Tom Clegg
* See [[Coding Standards#Git setup|Git setup]] below
118 4 Ward Vandewege
119 6 Tom Clegg
No inline comments: @this = !desired; # we don't want to do it.@
120 4 Ward Vandewege
121 6 Tom Clegg
No long (>80 column) lines, except in rare cases when the alternative is really clunky.
122 6 Tom Clegg
123 4 Ward Vandewege
No whitespace at the end of lines. Make git-diff show you:
124 5 Ward Vandewege
125 5 Ward Vandewege
  git config color.diff.whitespace "red reverse"
126 6 Tom Clegg
git diff --check
127 1 Tom Clegg
128 13 Tom Clegg
h2. What to include
129 13 Tom Clegg
130 1 Tom Clegg
No commented-out blocks of code that have been replaced or obsoleted.
131 1 Tom Clegg
132 1 Tom Clegg
* It is in the git history if we want it back.
133 1 Tom Clegg
* 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.
134 1 Tom Clegg
135 1 Tom Clegg
No commented-out debug statements.
136 1 Tom Clegg
137 1 Tom Clegg
* 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"@
138 1 Tom Clegg
139 13 Tom Clegg
h2. Style mismatch
140 13 Tom Clegg
141 1 Tom Clegg
Adopt indentation style of surrounding lines or (when starting a new file) the nearest existing source code in this tree/language.
142 1 Tom Clegg
143 1 Tom Clegg
If you fix up existing indentation/formatting, do that in a separate commit.
144 1 Tom Clegg
* If you bundle formatting changes with functional changes, it makes functional changes hard to find in the diff.
145 1 Tom Clegg
146 13 Tom Clegg
h2. Go
147 13 Tom Clegg
148 13 Tom Clegg
gofmt, golint, etc., and https://github.com/golang/go/wiki/CodeReviewComments
149 13 Tom Clegg
150 13 Tom Clegg
h2. Ruby
151 13 Tom Clegg
152 13 Tom Clegg
https://github.com/bbatsov/ruby-style-guide
153 13 Tom Clegg
154 1 Tom Clegg
h2. Python
155 13 Tom Clegg
156 13 Tom Clegg
PEP-8.
157 12 Tom Clegg
158 12 Tom Clegg
Tell Emacs you don't want a blank line at the end of a multiline docstring.
159 12 Tom Clegg
160 12 Tom Clegg
    (setq python-fill-docstring-style 'pep-257-nn)
161 12 Tom Clegg
162 11 Brett Smith
h2. JavaScript
163 11 Brett Smith
164 20 Tom Clegg
Follow the Airbnb Javascript coding style guide unless otherwise stated:
165 14 Tom Morris
https://github.com/airbnb/javascript
166 20 Tom Clegg
167 20 Tom Clegg
We already have 4-space indents everywhere, though, so do that.
168 20 Tom Clegg
169 14 Tom Morris
170 7 Tom Clegg
h2. Git setup
171 6 Tom Clegg
172 7 Tom Clegg
Configure git to prevent you from committing whitespace errors.
173 1 Tom Clegg
174 6 Tom Clegg
<pre>
175 7 Tom Clegg
git config --global core.whitespace tab-in-indent,trailing-space
176 1 Tom Clegg
git config --global apply.whitespace error
177 17 Tom Clegg
</pre>
178 17 Tom Clegg
179 17 Tom Clegg
Add a DCO sign-off to the default commit message.
180 17 Tom Clegg
181 17 Tom Clegg
<pre>
182 17 Tom Clegg
cd .../arvados
183 17 Tom Clegg
printf '\n\nArvados-DCO-1.1-Signed-off-by: %s <%s>\n' "$(git config user.name)" "$(git config user.email)" >~/.arvados-dco.txt
184 17 Tom Clegg
git config commit.template ~/.arvados-dco.txt
185 17 Tom Clegg
</pre>
186 17 Tom Clegg
187 17 Tom Clegg
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.
188 17 Tom Clegg
189 17 Tom Clegg
<pre>
190 17 Tom Clegg
cd .../arvados
191 17 Tom Clegg
cat >.git/hooks/prepare-commit-message <<'EOF'
192 17 Tom Clegg
#!/bin/sh
193 17 Tom Clegg
194 17 Tom Clegg
case "$2,$3" in
195 17 Tom Clegg
    merge,)
196 17 Tom Clegg
        br=$(head -n1 ${1})
197 17 Tom Clegg
        n=$(echo "${br}" | egrep -o '[0-9]+')
198 17 Tom Clegg
        exec >${1}
199 17 Tom Clegg
        echo "${br}"
200 17 Tom Clegg
        echo
201 17 Tom Clegg
        echo "refs #${n}"
202 17 Tom Clegg
        echo
203 17 Tom Clegg
        echo "Arvados-DCO-1.1-Signed-off-by: ${GIT_AUTHOR_NAME} <${GIT_AUTHOR_EMAIL}>"
204 17 Tom Clegg
        ;;
205 17 Tom Clegg
    *)
206 17 Tom Clegg
        ;;
207 17 Tom Clegg
esac
208 17 Tom Clegg
EOF
209 17 Tom Clegg
chmod +x .git/hooks/prepare-commit-message
210 6 Tom Clegg
</pre>