Project

General

Profile

Coding Standards » History » Version 20

Tom Clegg, 09/29/2017 02:01 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 16 Tom Clegg
71 13 Tom Clegg
h2. Source code formatting
72 1 Tom Clegg
73 13 Tom Clegg
(Unless otherwise specified by style guide...)
74
75 10 Tom Clegg
No TAB characters in source files. "Except go programs.":https://golang.org/cmd/gofmt/
76 1 Tom Clegg
77 6 Tom Clegg
* Emacs: add to @~/.emacs@ &rarr; @(setq-default indent-tabs-mode nil)@
78
* Vim: add to @~/.vimrc@ &rarr; @:set expandtab@
79 8 Tom Clegg
* See [[Coding Standards#Git setup|Git setup]] below
80 4 Ward Vandewege
81 6 Tom Clegg
No inline comments: @this = !desired; # we don't want to do it.@
82 4 Ward Vandewege
83 6 Tom Clegg
No long (>80 column) lines, except in rare cases when the alternative is really clunky.
84
85 4 Ward Vandewege
No whitespace at the end of lines. Make git-diff show you:
86 5 Ward Vandewege
87
  git config color.diff.whitespace "red reverse"
88 6 Tom Clegg
git diff --check
89 1 Tom Clegg
90 13 Tom Clegg
h2. What to include
91
92 1 Tom Clegg
No commented-out blocks of code that have been replaced or obsoleted.
93
94
* It is in the git history if we want it back.
95
* 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.
96
97
No commented-out debug statements.
98
99
* 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"@
100
101 13 Tom Clegg
h2. Style mismatch
102
103 1 Tom Clegg
Adopt indentation style of surrounding lines or (when starting a new file) the nearest existing source code in this tree/language.
104
105
If you fix up existing indentation/formatting, do that in a separate commit.
106
* If you bundle formatting changes with functional changes, it makes functional changes hard to find in the diff.
107
108 13 Tom Clegg
h2. Go
109
110
gofmt, golint, etc., and https://github.com/golang/go/wiki/CodeReviewComments
111
112
h2. Ruby
113
114
https://github.com/bbatsov/ruby-style-guide
115
116 1 Tom Clegg
h2. Python
117 13 Tom Clegg
118
PEP-8.
119 12 Tom Clegg
120
Tell Emacs you don't want a blank line at the end of a multiline docstring.
121
122
    (setq python-fill-docstring-style 'pep-257-nn)
123
124 11 Brett Smith
h2. JavaScript
125
126 20 Tom Clegg
Follow the Airbnb Javascript coding style guide unless otherwise stated:
127 14 Tom Morris
https://github.com/airbnb/javascript
128 20 Tom Clegg
129
We already have 4-space indents everywhere, though, so do that.
130
131 14 Tom Morris
132 7 Tom Clegg
h2. Git setup
133 6 Tom Clegg
134 7 Tom Clegg
Configure git to prevent you from committing whitespace errors.
135 1 Tom Clegg
136 6 Tom Clegg
<pre>
137 7 Tom Clegg
git config --global core.whitespace tab-in-indent,trailing-space
138 1 Tom Clegg
git config --global apply.whitespace error
139 17 Tom Clegg
</pre>
140
141
Add a DCO sign-off to the default commit message.
142
143
<pre>
144
cd .../arvados
145
printf '\n\nArvados-DCO-1.1-Signed-off-by: %s <%s>\n' "$(git config user.name)" "$(git config user.email)" >~/.arvados-dco.txt
146
git config commit.template ~/.arvados-dco.txt
147
</pre>
148
149
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.
150
151
<pre>
152
cd .../arvados
153
cat >.git/hooks/prepare-commit-message <<'EOF'
154
#!/bin/sh
155
156
case "$2,$3" in
157
    merge,)
158
        br=$(head -n1 ${1})
159
        n=$(echo "${br}" | egrep -o '[0-9]+')
160
        exec >${1}
161
        echo "${br}"
162
        echo
163
        echo "refs #${n}"
164
        echo
165
        echo "Arvados-DCO-1.1-Signed-off-by: ${GIT_AUTHOR_NAME} <${GIT_AUTHOR_EMAIL}>"
166
        ;;
167
    *)
168
        ;;
169
esac
170
EOF
171
chmod +x .git/hooks/prepare-commit-message
172 6 Tom Clegg
</pre>