Permalink
Newer
100644
219 lines (174 sloc)
7.51 KB
4
We're following the
5
[Google Go Code Review](https://code.google.com/p/go-wiki/wiki/CodeReviewComments)
6
fairly closely. In particular, you want to watch out for proper
7
punctuation and capitalization in comments. We use two-space indents
8
in non-Go code (in Go, we follow `gofmt` which indents with
11
### Line Length
12
Format your code assuming it will be read in a window 100 columns wide.
13
Wrap code at 100 characters and comments at 80 unless doing so makes the
17
When wrapping function signatures that do not fit on one line,
18
put the name, arguments, and return types on separate lines, with the closing `)`
19
at the same indentation as `func` (this helps visually separate the indented
20
arguments from the indented function body). Example:
21
```go
22
func (s *someType) myFunctionName(
23
arg1 somepackage.SomeArgType, arg2 int, arg3 somepackage.SomeOtherType,
24
) (somepackage.SomeReturnType, error) {
25
...
26
}
27
```
28
29
If the arguments list is too long to fit on a single line, switch to one
30
argument per line:
31
```go
32
func (s *someType) myFunctionName(
33
arg1 somepackage.SomeArgType,
34
arg2 int,
35
arg3 somepackage.SomeOtherType,
36
) (somepackage.SomeReturnType, error) {
37
...
38
}
39
```
40
41
If the return types need to be wrapped, use the same rules:
42
```go
43
func (s *someType) myFunctionName(
44
arg1 somepackage.SomeArgType, arg2 somepackage.SomeOtherType,
45
) (
46
somepackage.SomeReturnType,
47
somepackage.SomeOtherType,
48
error,
49
) {
50
...
51
}
52
```
53
54
Exception when omitting repeated types for consecutive arguments:
55
short and related arguments (e.g. `start, end int64`) should either go on the same line
56
or the type should be repeated on each line -- no argument should appear by itself
57
on a line with no type (confusing and brittle when edited).
59
### Inline comments for arguments to function calls
60
61
A code reader encountering a function call should be able to intuit what all
62
the arguments to the call represent. Whenever it wouldn't be otherwise clear
63
what the value used as an argument represents (for example, from the variable's
64
name if a variable is used or from the type name if a struct literal is used),
65
consider annotating it with an inline comment specifying the respective
66
parameter's name. Particularly, consider doing this for literals of "basic"
67
types (boolean, numeric, string types, whether the type is predeclared or not)
68
and for `nil` identifiers, as they are frequently not suggestive enough of what
69
they represent.
70
71
For example:
72
73
```
74
intentsToEvalResult(externalIntents, args, false /* alwaysReturn */)
75
76
monitor := mon.MakeMonitor(
77
"in-mem temp storage",
78
mon.MemoryResource,
79
nil, /* curCount */
80
nil, /* maxHist */
81
1024*1024, /* increment */
82
maxSizeBytes/10, /* noteworthy */
83
)
84
```
85
86
Note: For `bool` constants, like for all literals, the comment should indicate
87
the name of the parameter and does not depend on the argument value.
88
*Do not* put a bang in the comment when commenting the `false` constant. Also,
89
do not adapt the comment to negate the name of the parameter. For example:
90
91
```
92
func endTxn(commit bool){}
93
94
OK: endTxn(false /* commit */)
95
NOT OK: endTxn(false /* !commit */)
96
NOT OK: endTxn(false /* abort */)
97
// If you want to add an explanation to an argument, a suggested style is to
98
// include both the param name and the explanation with a dash between them:
99
OK: endTxn(false /* commit - we abort as we concluded above that we can't commit */)
100
```
101
102
#### Try to avoid `bool` parameters
103
104
`bool` arguments to functions are often dubious things, as they hint to code that
105
essentially reads like:
106
107
```
108
func doSomething(shouldDoX bool) {
109
if shouldDoX {
110
doX()
111
} else {
112
doY()
113
}
114
}
115
```
116
117
This is not always the case. However, in cases where that is a fair assessment
118
of the situation, consider whether the `doSomething` function should exist at
119
all.
120
In cases where the `bool` in question, along with other arguments, acts as a
121
"knob" to the function consider replacing it with some type of "configuration"
122
struct (for examples, see [Dave Cheney's treatment of the
123
topic](https://dave.cheney.net/2014/10/17/functional-options-for-friendly-apis)).
124
In situations where there's a single `bool` param or the situation is less
125
clear-cut, consider replacing the `bool` in question with an enum. For example:
126
127
```
128
type EndTxnAction bool
129
130
const (
131
Commit EndTxnAction = false
132
Abort = true
133
)
134
135
func endTxn(action EndTxnAction) {}
136
```
137
is better than
138
```
139
func endTxn(commit bool) {}
140
```
141
142
### fmt Verbs
143
144
Prefer the most specific verb for your use. In other words, prefer to avoid %v
145
when possible. However, %v is to be used when formatting bindings which might
146
be nil and which do not already handle nil formatting. Notably, nil errors
147
formatted as %s will render as "%!s(<nil>)" while nil errors formatted as %v
148
will render as "<nil>". Therefore, prefer %v when formatting errors which are
149
not known to be non-nil.
150
151
### Distinguishing user errors from internal errors
152
153
When creating an error for something that the user did wrong (and thus isn't
154
indicative of an unexpected situation in our code), use `fmt.Errorf()` to create
155
the error.
156
157
When creating an error for an unexpected situation, use methods from the
158
[`errors` package that we use](https://github.com/pkg/errors), such as
159
`errors.New()`, `errors.Errorf()`, `errors.Wrap()`, or `errors.Wrapf()`.
160
161
The reason for this distinction is somewhat historical (#7424), but maintaining
162
it will help us immensely if we ever switch to using new error types for the
163
different situations.
164
165
## Go implementation of SQL
166
167
When defining the result column labels of statements or virtual
168
tables, apply the following general principles:
169
170
- the casing of column labels must be consistent.
171
172
- the same concept in different statements should be named using the
173
same word.
174
175
For example, `variable` and `value` corresponds between SHOW ALL
176
CLUSTER SETTINGS and SHOW SESSION ALL.
177
178
- the labels must be usable by a surrounding query without mandating
179
quotes.
180
181
For example, `start_key` instead of `"Start Key"`. Also, avoid SQL
182
keywords which would also need to be quoted: `table_name` instead of
183
`"table"`, `index_name` instead of `"index"`, etc.
184
185
Never name a column `"user"`! The risk of mistake with the syntax
186
special form `user` is too high. Use `user_name` instead.
187
188
- use underscores to separate words, for consistency with
189
`information_schema`.
190
191
For example, `start_key` instead of `startkey`
192
193
- avoid abbreviations unless most uses of the word in spoken English
194
use the abbreviation already.
195
196
For example, `id` for "identifier" is OK, but `loc` for "location"
197
is not.
198
199
- for columns akin to "primary keys" (that identify the object being
200
described by the remaining columns), words whose meaning is highly
201
overloaded must be disambiguated in the label.
202
203
For example, `zone_id`, `job_id`, `table_id` instead of just `id`
204
everywhere. `table_name`, `column_name`, etc instead of `name`
205
everywhere.
206
207
- for database objects that can be referred to by two or more types of
208
handles (e.g. by-ID or by-name), the column label must disambiguate
209
which type of handle is being reported. This makes it possible to
210
later report the other handles in additional columns.
211
212
For example, `table_name` instead of `table`, so that `table_id` can
213
be added later.
214
215
- labels that reproduce data already present in an
216
`information_schema` table or a prior `SHOW` statement should use
217
the same labels as the `information_schema` table or `SHOW`
218
statement, provided it matches the principles above already.