Merge pull request #6228 from guggero/contribution-guidelines

docs: rewrite and regroup formatting rules [skip ci]
This commit is contained in:
Oliver Gugger 2022-02-03 10:02:02 +01:00 committed by GitHub
commit 04bbbea9a7
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23

View file

@ -9,9 +9,7 @@
1. [Model Git Commit Messages](#model-git-commit-messages) 1. [Model Git Commit Messages](#model-git-commit-messages)
1. [Ideal Git Commit Structure](#ideal-git-commit-structure) 1. [Ideal Git Commit Structure](#ideal-git-commit-structure)
1. [Sign Your Git Commits](#sign-your-git-commits) 1. [Sign Your Git Commits](#sign-your-git-commits)
1. [Code Spacing](#code-spacing) 1. [Code Spacing and formatting](#code-spacing-and-formatting)
1. [Protobuf Compilation](#protobuf-compilation)
1. [Additional Style Constraints On Top of gofmt](#additional-style-constraints-on-top-of-gofmt)
1. [Pointing to Remote Dependent Branches in Go Modules](#pointing-to-remote-dependent-branches-in-go-modules) 1. [Pointing to Remote Dependent Branches in Go Modules](#pointing-to-remote-dependent-branches-in-go-modules)
1. [Use of Log Levels](#use-of-log-levels) 1. [Use of Log Levels](#use-of-log-levels)
5. [Code Approval Process](#code-approval-process) 5. [Code Approval Process](#code-approval-process)
@ -397,7 +395,21 @@ Use `git log` with `--show-signature`,
You can also pass the `--show-signature` option to `git show` to check a single You can also pass the `--show-signature` option to `git show` to check a single
commit. commit.
## Code Spacing ## Code Spacing and formatting
### Why this emphasis on formatting?
Code in general (and Open Source code specifically) is _read_ by developers many
more times during its lifecycle than it is modified. With this fact in mind, the
Golang language was designed for readability (among other goals).
While the enforced formatting of `go fmt` and some best practices already
eliminates many discussions, the resulting code can still look and feel very
differently between different developers.
We aim to enforce a few additional rules to unify the look and feel of all code
in `lnd` to help improve the overall readability.
### Spacing
Blocks of code within `lnd` should be segmented into logical stanzas of Blocks of code within `lnd` should be segmented into logical stanzas of
operation. Such spacing makes the code easier to follow at a skim, and reduces operation. Such spacing makes the code easier to follow at a skim, and reduces
@ -450,43 +462,83 @@ statements and select statements.
**WRONG** **WRONG**
```go ```go
switch { switch {
case a: case a:
<code block> <code block>
case b: case b:
<code block> <code block>
case c: case c:
<code block> <code block>
case d: case d:
<code block> <code block>
default: default:
<code block> <code block>
} }
``` ```
**RIGHT** **RIGHT**
```go ```go
switch { switch {
// Brief comment detailing instances of this case (repeat below). // Brief comment detailing instances of this case (repeat below).
case a: case a:
<code block> <code block>
case b: case b:
<code block> <code block>
case c: case c:
<code block> <code block>
case d: case d:
<code block> <code block>
default: default:
<code block> <code block>
} }
``` ```
If one is forced to wrap lines of function arguments that exceed the 80 ### Additional Style Constraints
character limit, then identation must be kept on the following lines. Also,
lines should not end with an open open parenthesis. Before a PR is submitted, the proposer should ensure that the file passes the
set of linting scripts run by `make lint`. These include `gofmt`. In addition
to `gofmt` we've opted to enforce the following style guidelines.
**80 character line length**: ALL columns (on a best effort basis) should be
wrapped to 80 line columns.
Editors should be set to treat a **tab as 8 spaces**.
**WRONG**
```go
myKey := "0214cd678a565041d00e6cf8d62ef8add33b4af4786fb2beb87b366a2e151fcee7"
```
**RIGHT**
```go
myKey := "0214cd678a565041d00e6cf8d62ef8add33b4af4786fb2beb87b366a2e1" +
"51fcee7"
```
**Wrapping long function calls**: When wrapping a line that contains a function
call as the unwrapped line exceeds the column limit, the close paren should be
placed on its own line. Additionally, all arguments should begin in a new line
after the open paren.
**WRONG**
```go
value, err := bar(a,
a, b, c)
```
**RIGHT**
```go
value, err := bar(
a, a, b, c,
)
```
**Wrapping long function definitions**: If one is forced to wrap lines of
function arguments that exceed the 80 character limit, then indentation must be
kept on the following lines. Also, lines should not end with an open
parenthesis if the function definition isn't finished yet.
**WRONG** **WRONG**
```go ```go
@ -494,83 +546,74 @@ func foo(a, b, c,
) (d, error) { ) (d, error) {
func bar(a, b, c) ( func bar(a, b, c) (
d, error, d, error,
) { ) {
func baz(a, b, c) ( func baz(a, b, c) (
d, error) { d, error) {
``` ```
**RIGHT** **RIGHT**
```go ```go
func foo(a, b, func foo(a, b,
c) (d, error) { c) (d, error) {
func baz(a, b, c) (d, func baz(a, b, c) (d,
error) { error) {
``` ```
If a function declaration spans multiple lines the body should start with an If a function declaration spans multiple lines the body should start with an
empty line. empty line to help visually distinguishing the two elements.
**WRONG** **WRONG**
```go ```go
func foo(a, b, c, func foo(a, b, c,
d, e) error { d, e) error {
var a int var a int
} }
``` ```
**RIGHT** **RIGHT**
```go ```go
func foo(a, b, c, func foo(a, b, c,
d, e) error { d, e) error {
var a int var a int
} }
``` ```
## Protobuf Compilation
The `lnd` project uses `protobuf`, and its extension [`gRPC`](www.grpc.io) in
several areas and as the primary RPC interface. In order to ensure uniformity
of all protos checked, in we require that all contributors pin against the
_exact same_ version of `protoc`. As of the writing of this article, the `lnd`
project uses [v3.4.0](https://github.com/google/protobuf/releases/tag/v3.4.0)
of `protoc`.
For detailed instructions on how to compile modifications to `lnd`'s `protobuf`
definitions, check out the [lnrpc README](https://github.com/lightningnetwork/lnd/blob/master/lnrpc/README.md).
## Additional Style Constraints On Top of `gofmt`
Before a PR is submitted, the proposer should ensure that the file passes the
set of linting scripts run by `make lint`. These include `gofmt`. In addition
to `gofmt` we've opted to enforce the following style guidelines.
* ALL columns (on a best effort basis) should be wrapped to 80 line columns.
Editors should be set to treat a tab as 8 spaces.
* When wrapping a line that contains a function call as the unwrapped line
exceeds the column limit, the close paren should be placed on its own
line. Additionally, all arguments should begin in a new line after the
open paren.
**WRONG**
```go
value, err := bar(a,
a, b, c)
```
**RIGHT**
```go
value, err := bar(
a, a, b, c,
)
```
**Note that the above guidelines don't apply to log or error messages.** For **Note that the above guidelines don't apply to log or error messages.** For
log and error messages, committers should attempt to minimize the number of log and error messages, committers should attempt to minimize the number of
lines utilized, while still adhering to the 80-character column limit. lines utilized, while still adhering to the 80-character column limit. For
example:
**WRONG**
```go
return fmt.Errorf(
"this is a long error message with a couple (%d) place holders",
len(things),
)
log.Debugf(
"Something happened here that we need to log: %v",
longVariableNameHere,
)
```
**RIGHT**
```go
return fmt.Errorf("this is a long error message with a couple (%d) place "+
"holders", len(things))
log.Debugf("Something happened here that we need to log: %v",
longVariableNameHere)
```
This helps to visually distinguish those formatting statements (where nothing
of consequence happens except for formatting an error message or writing
to a log) from actual method or function calls. This compact formatting should
be used for calls to formatting functions like `fmt.Errorf`,
`log.(Trace|Debug|Info|Warn|Error)f` and `fmt.Printf`.
But not for statements that are important for the flow or logic of the code,
like `require.NoErrorf()`.
## Pointing to Remote Dependent Branches in Go Modules ## Pointing to Remote Dependent Branches in Go Modules