Skip to content

Commit

Permalink
docs: polish code style document for pouch
Browse files Browse the repository at this point in the history
Signed-off-by: Allen Sun <[email protected]>
  • Loading branch information
allencloud committed May 8, 2018
1 parent 3d2c538 commit 0001ed7
Show file tree
Hide file tree
Showing 2 changed files with 193 additions and 14 deletions.
2 changes: 1 addition & 1 deletion .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ jobs:
- run:
name: use markdownlint v0.4.0 to lint markdown file (https://github.com/markdownlint/markdownlint)
command: |
find ./ -name "*.md" | grep -v vendor | grep -v extra | grep -v commandline | grep -v .github | grep -v swagger | grep -v api | xargs mdl -r ~MD013,~MD024,~MD029,~MD033,~MD036
find ./ -name "*.md" | grep -v vendor | grep -v extra | grep -v commandline | grep -v .github | grep -v swagger | grep -v api | xargs mdl -r ~MD010,~MD013,~MD024,~MD029,~MD033,~MD036
code-check:
docker:
Expand Down
205 changes: 192 additions & 13 deletions docs/contributions/code_styles.md
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
# Code Style

Code style is a set of rules or guidelines when writing source codes of a software project. Following particular code style will definitely help contributors to read and understand source codes. In addition, it will help to avoid introducing errors as well.
Code style is a set of rules or guidelines when writing source codes of a software project. Following particular code style will definitely help contributors to read and understand source codes very well. In addition, it will help to avoid introducing errors as well.

## Code Style Tools

Project Pouch is written in Golang. And currently we use three tools to help conform code styles of this project. These three tools are:
Project Pouch is written in Golang. And currently we use three tools to help conform code styles in this project. These three tools are:

* [gofmt](https://golang.org/cmd/gofmt)
* [golint](https://github.com/golang/lint)
Expand All @@ -14,19 +14,198 @@ And all these tools are used in [Makefile](../../Makefile).

## Code Review Comments

When collaborating in Pouch project, we follow the style from [Go Code Review Commnets](https://github.com/golang/go/wiki/CodeReviewComments). Before contributing, we treat this as a must-read.
When collaborating in Pouch project, we follow the style from [Go Code Review Comments](https://github.com/golang/go/wiki/CodeReviewComments). Before contributing, we treat this as a must-read.

## Additional Style Rules

For a project, existing tools and rules may not be sufficient. To align more in styles, we recommend contributors taking a thorough look at the following additional style rules:

* When constructing a struct, if comments needed for fields in struct, keep a blank line between fields;
* When defining interface functions, we should always explicitly add formal parameter, and this helps a lot to code readability;
* When defining interface functions, if comments needed for functions, keep a blank line between functions;
* When importing packages, to improve readabilities, we should import package by sequence: system packages, project's own packages and third-party packages. And we should keep a blank line among these three kinds of packages;
* Const object should be declared at the beginning of the go file, following package name and importing;
* When generating error in an action failure, we should generally use the way of `fmt.Errorf("failed to do something: %v", err)`;
* No matter log or error, first letter of the message must be lower-case;
* When occurring nesting errors, we recommend first considering using package `github.com/pkg/errors`;
* Every comment should begin with `//` plus a space, and please don't forget the whitespace, and end with a `.`;
* We should take `DRY(Don't Repeat Yourself)` into more consideration.
### RULE001 - Add blank line between field's comments

When constructing a struct, if comments needed for fields in struct, keep a blank line between fields. The ecouraged way is as following:

``` golang
// correct example
// ContainerManager is the default implement of interface ContainerMgr.
type ContainerManager struct {
// Store stores containers in Backend store.
// Element operated in store must has a type of *ContainerMeta.
// By default, Store will use local filesystem with json format to store containers.
Store *meta.Store

// Client is used to interact with containerd.
Client ctrd.APIClient

// NameToID stores relations between container's name and ID.
// It is used to get container ID via container name.
NameToID *collect.SafeMap
......
}
```

Rather than:

```golang
// wrong example
// ContainerManager is the default implement of interface ContainerMgr.
type ContainerManager struct {
// Store stores containers in Backend store.
// Element operated in store must has a type of *ContainerMeta.
// By default, Store will use local filesystem with json format to store containers.
Store *meta.Store
// Client is used to interact with containerd.
Client ctrd.APIClient
// NameToID stores relations between container's name and ID.
// It is used to get container ID via container name.
NameToID *collect.SafeMap
......
}
```

### RULE003 - Add parameter name in interface definition

When defining interface functions, we should always explicitly add formal parameters, and this helps a lot to code readability. For example, the following way are preferred:

``` golang
// correct example
// ContainerMgr is an interface to define all operations against container.
type ContainerMgr interface {
// Start a container.
Start(ctx context.Context, id, detachKeys string) error

// Stop a container.
Stop(ctx context.Context, name string, timeout int64) error
......
}
```

However, missing formal parameter's name would make interface unreadable, since we would never know what the parameter's real meaning unless turning to one implementation of this interface:

``` golang
// wrong example
type ContainerMgr interface {
// Start a container.
Start(context.Context, string, string) error

// Stop a container.
Stop(context.Context, string, int64) error
......
}

```

In addition, a blank line between function's comments is encouraged to make interface more readable.

### RULE004 - Import Packages

When importing packages, to improve readabilities, we should import package by sequence:

* Golang's built-in system packages;
* project's own packages;
* third-party packages.

And we should keep a blank line among these three kinds of packages like the following:

``` golang
import (
"fmt"
"strconv"
"strings"

"github.com/alibaba/pouch/apis/types"
"github.com/alibaba/pouch/pkg/errtypes"
"github.com/alibaba/pouch/pkg/meta"
"github.com/alibaba/pouch/pkg/randomid"

"github.com/opencontainers/selinux/go-selinux/label"
"github.com/pkg/errors"
)
```

### RULE005 - Variable declaration position

Variable object should be declared at the beginning of the go file following package name and importing.

### RULE006 - Generation of action failure

When generating error in one function execution failure, we should generally use the following way to append string "failed to do something" and the specific err instance to construct a new error:

``` golang
fmt.Errorf("failed to do something: %v", err)
```

When an err could be thrown out, please remember to add it in the error construction.

### RULE007 - Return fast to ident less

Pouch encourages contributors to take advantages of `return fast` to simply source code and ident less. For example, the following codes are discouraged:

``` golang
// wrong example
if retry {
if t, err := calculateSleepTime(d); err == nil {
time.Sleep(t)
times++
return retryLoad()
}
return fmt.Errorf("failed to calculate timeout: %v", err)
}
return nil
```

In code above, there are some idents which can be avoided. The ecouraged way is like the following:

``` golang
// correct example
if !retry {
return nil
}

t, err := calculateSleepTime(d);
if err != nil {
return fmt.Errorf("failed to calculate timeout: %v", err)
}

time.Sleep(t)
times++

return retryLoad()
```

### RULE008 - Lowercase log and error

No matter log or error, first letter of the message must be lower-case. So, `logrus.Debugf("failed to add list: %v", err)` is encouraged. And `logrus.Debugf("Failed to add list: %v", err)` is not perferred.

### RULE009 - Nested errors

When occurring nesting errors, we recommend first considering using package `github.com/pkg/errors`.

### RULE010 - Comment correctly

Every comment must begin with `//` plus a whitespace no matter for a variable, struct, function, code block and anything else. Please don't forget the whitespace, and end up all the sentence with a `.`. In addition, it is encouraged to use third person singular to polish the majority of function's comments. For example, the following way

```golang
// wrong example
// ExecContainer execute a process in container.
func (c *Client) ExecContainer(ctx context.Context, process *Process) error {
......
}
```

could be polished to be `executes` rather than `execute`:

```golang
// correct example
// ExecContainer executes a process in container.
func (c *Client) ExecContainer(ctx context.Context, process *Process) error {
......
}
```

### RULE011 - Always remember DRY

We should take `DRY(Don't Repeat Yourself)` into consideration when adding anything.

### RULE012 - Welcome to your addition

If you think much more practical code styles should be introduced in Pouch. Please submit a pull request to make this better.

0 comments on commit 0001ed7

Please sign in to comment.