Coding Standards » History » Version 17

Tom Clegg, 06/21/2017 07:54 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 9 Tom Clegg
Refer to a story number in each commit comment.
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 9 Tom Clegg
* @closes #1234@, or
21 9 Tom Clegg
* @refs #1234@
22 9 Tom Clegg
23 1 Tom Clegg
Use descriptive commit comments.
24 1 Tom Clegg
25 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.
26 9 Tom Clegg
* Good: "1234: Support use of spaces in filenames."
27 9 Tom Clegg
* Good: "1234: Fix crash when user_id is nil."
28 1 Tom Clegg
* Less good: "Add some controller methods." (What do they do?)
29 1 Tom Clegg
* Less good: "More progress on UI branch." (What is different?)
30 1 Tom Clegg
* Less good: "Incorporate Tom's suggestions." (Who cares whose suggestions -- what changed?)
31 1 Tom Clegg
32 1 Tom Clegg
If further background or explanation is needed, separate it from the summary with a blank line.
33 1 Tom Clegg
34 1 Tom Clegg
* Example: "Users found it confusing that the boxes had different colors even though they represented the same kinds of things."
35 1 Tom Clegg
36 16 Tom Clegg
*Every commit* must have a DCO sign-off. See [[Developer Certificate Of Origin]].
37 16 Tom Clegg
38 16 Tom Clegg
* Example: <code>Arvados-DCO-1.1-Signed-off-by: Joe Smith <joe.smith@example.com></code>
39 16 Tom Clegg
40 13 Tom Clegg
h2. Source code formatting
41 1 Tom Clegg
42 13 Tom Clegg
(Unless otherwise specified by style guide...)
43 13 Tom Clegg
44 10 Tom Clegg
No TAB characters in source files. "Except go programs.":https://golang.org/cmd/gofmt/
45 1 Tom Clegg
46 6 Tom Clegg
* Emacs: add to @~/.emacs@ &rarr; @(setq-default indent-tabs-mode nil)@
47 6 Tom Clegg
* Vim: add to @~/.vimrc@ &rarr; @:set expandtab@
48 8 Tom Clegg
* See [[Coding Standards#Git setup|Git setup]] below
49 4 Ward Vandewege
50 6 Tom Clegg
No inline comments: @this = !desired; # we don't want to do it.@
51 4 Ward Vandewege
52 6 Tom Clegg
No long (>80 column) lines, except in rare cases when the alternative is really clunky.
53 6 Tom Clegg
54 4 Ward Vandewege
No whitespace at the end of lines. Make git-diff show you:
55 5 Ward Vandewege
56 5 Ward Vandewege
  git config color.diff.whitespace "red reverse"
57 6 Tom Clegg
git diff --check
58 1 Tom Clegg
59 13 Tom Clegg
h2. What to include
60 13 Tom Clegg
61 1 Tom Clegg
No commented-out blocks of code that have been replaced or obsoleted.
62 1 Tom Clegg
63 1 Tom Clegg
* It is in the git history if we want it back.
64 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.
65 1 Tom Clegg
66 1 Tom Clegg
No commented-out debug statements.
67 1 Tom Clegg
68 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"@
69 1 Tom Clegg
70 13 Tom Clegg
h2. Style mismatch
71 13 Tom Clegg
72 1 Tom Clegg
Adopt indentation style of surrounding lines or (when starting a new file) the nearest existing source code in this tree/language.
73 1 Tom Clegg
74 1 Tom Clegg
If you fix up existing indentation/formatting, do that in a separate commit.
75 1 Tom Clegg
* If you bundle formatting changes with functional changes, it makes functional changes hard to find in the diff.
76 1 Tom Clegg
77 13 Tom Clegg
h2. Go
78 13 Tom Clegg
79 13 Tom Clegg
gofmt, golint, etc., and https://github.com/golang/go/wiki/CodeReviewComments
80 13 Tom Clegg
81 13 Tom Clegg
h2. Ruby
82 13 Tom Clegg
83 13 Tom Clegg
https://github.com/bbatsov/ruby-style-guide
84 13 Tom Clegg
85 1 Tom Clegg
h2. Python
86 13 Tom Clegg
87 13 Tom Clegg
PEP-8.
88 12 Tom Clegg
89 12 Tom Clegg
Tell Emacs you don't want a blank line at the end of a multiline docstring.
90 12 Tom Clegg
91 12 Tom Clegg
    (setq python-fill-docstring-style 'pep-257-nn)
92 12 Tom Clegg
93 11 Brett Smith
h2. JavaScript
94 11 Brett Smith
95 15 Tom Morris
"Always use 3 equals unless you have a good reason to use 2.":https://github.com/airbnb/javascript#comparison--eqeqeq
96 14 Tom Morris
"Use 2 spaces for indents":https://github.com/airbnb/javascript#whitespace--spaces
97 14 Tom Morris
98 14 Tom Morris
Follow the Airbnb Javascript coding style guide unless otherwise stated (the rules above are for emphasis, not exceptions)
99 14 Tom Morris
https://github.com/airbnb/javascript
100 14 Tom Morris
101 7 Tom Clegg
h2. Git setup
102 6 Tom Clegg
103 7 Tom Clegg
Configure git to prevent you from committing whitespace errors.
104 1 Tom Clegg
105 6 Tom Clegg
<pre>
106 7 Tom Clegg
git config --global core.whitespace tab-in-indent,trailing-space
107 1 Tom Clegg
git config --global apply.whitespace error
108 17 Tom Clegg
</pre>
109 17 Tom Clegg
110 17 Tom Clegg
Add a DCO sign-off to the default commit message.
111 17 Tom Clegg
112 17 Tom Clegg
<pre>
113 17 Tom Clegg
cd .../arvados
114 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
115 17 Tom Clegg
git config commit.template ~/.arvados-dco.txt
116 17 Tom Clegg
</pre>
117 17 Tom Clegg
118 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.
119 17 Tom Clegg
120 17 Tom Clegg
<pre>
121 17 Tom Clegg
cd .../arvados
122 17 Tom Clegg
cat >.git/hooks/prepare-commit-message <<'EOF'
123 17 Tom Clegg
#!/bin/sh
124 17 Tom Clegg
125 17 Tom Clegg
case "$2,$3" in
126 17 Tom Clegg
    merge,)
127 17 Tom Clegg
        br=$(head -n1 ${1})
128 17 Tom Clegg
        n=$(echo "${br}" | egrep -o '[0-9]+')
129 17 Tom Clegg
        exec >${1}
130 17 Tom Clegg
        echo "${br}"
131 17 Tom Clegg
        echo
132 17 Tom Clegg
        echo "refs #${n}"
133 17 Tom Clegg
        echo
134 17 Tom Clegg
        echo "Arvados-DCO-1.1-Signed-off-by: ${GIT_AUTHOR_NAME} <${GIT_AUTHOR_EMAIL}>"
135 17 Tom Clegg
        ;;
136 17 Tom Clegg
    *)
137 17 Tom Clegg
        ;;
138 17 Tom Clegg
esac
139 17 Tom Clegg
EOF
140 17 Tom Clegg
chmod +x .git/hooks/prepare-commit-message
141 6 Tom Clegg
</pre>