Project

General

Profile

Coding Standards » History » Version 25

Ward Vandewege, 06/04/2018 08:47 PM
Add link to the git hook that enforces the copyright headers

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 24 Ward Vandewege
There is a helper script at https://github.com/curoverse/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
110 25 Ward Vandewege
The actual git hook that enforces the copyright headers lives at https://github.com/curoverse/arvados-dev/blob/master/git/hooks/check-copyright-headers.sh
111
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
PEP-8.
160 12 Tom Clegg
161
Tell Emacs you don't want a blank line at the end of a multiline docstring.
162
163
    (setq python-fill-docstring-style 'pep-257-nn)
164
165 11 Brett Smith
h2. JavaScript
166
167 20 Tom Clegg
Follow the Airbnb Javascript coding style guide unless otherwise stated:
168 14 Tom Morris
https://github.com/airbnb/javascript
169 20 Tom Clegg
170
We already have 4-space indents everywhere, though, so do that.
171
172 14 Tom Morris
173 7 Tom Clegg
h2. Git setup
174 6 Tom Clegg
175 7 Tom Clegg
Configure git to prevent you from committing whitespace errors.
176 1 Tom Clegg
177 6 Tom Clegg
<pre>
178 7 Tom Clegg
git config --global core.whitespace tab-in-indent,trailing-space
179 1 Tom Clegg
git config --global apply.whitespace error
180 17 Tom Clegg
</pre>
181
182
Add a DCO sign-off to the default commit message.
183
184
<pre>
185
cd .../arvados
186
printf '\n\nArvados-DCO-1.1-Signed-off-by: %s <%s>\n' "$(git config user.name)" "$(git config user.email)" >~/.arvados-dco.txt
187
git config commit.template ~/.arvados-dco.txt
188
</pre>
189
190
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.
191
192
<pre>
193
cd .../arvados
194
cat >.git/hooks/prepare-commit-message <<'EOF'
195
#!/bin/sh
196
197
case "$2,$3" in
198
    merge,)
199
        br=$(head -n1 ${1})
200
        n=$(echo "${br}" | egrep -o '[0-9]+')
201
        exec >${1}
202
        echo "${br}"
203
        echo
204
        echo "refs #${n}"
205
        echo
206
        echo "Arvados-DCO-1.1-Signed-off-by: ${GIT_AUTHOR_NAME} <${GIT_AUTHOR_EMAIL}>"
207
        ;;
208
    *)
209
        ;;
210
esac
211
EOF
212
chmod +x .git/hooks/prepare-commit-message
213 6 Tom Clegg
</pre>