From 53aa1ceba5cc1b3b7e818dcfa44daa310f51832a Mon Sep 17 00:00:00 2001 From: Callum Waters Date: Fri, 27 Mar 2020 16:26:39 +0100 Subject: [PATCH] create style guide and update contributors section (#4576) --- CONTRIBUTING.md | 16 ++++- README.md | 17 +++--- STYLE_GUIDE.md | 159 ++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 182 insertions(+), 10 deletions(-) create mode 100644 STYLE_GUIDE.md diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 2681b73ae..db4302056 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -191,9 +191,23 @@ easy to reference the pull request where a change was introduced. - ensure pull branch is based on a recent `master` - run `make test` to ensure that all tests pass -- squash merge pull request +- [squash](https://stackoverflow.com/questions/5189560/squash-my-last-x-commits-together-using-git) merge pull request - the `unstable` branch may be used to aggregate pull merges before fixing tests +### Git Commit Style + +We follow the [Go style guide on commit messages](https://tip.golang.org/doc/contribute.html#commit_messages). Write concise commits that start with the package name and have a description that finishes the sentence "This change modifies Tendermint to...". For example, + +\``` +cmd/debug: execute p.Signal only when p is not nil + +[potentially longer description in the body] + +Fixes #nnnn +\``` + +Each PR should have one commit once it lands on `master`; this can be accomplished by using the "squash and merge" button on Github. Be sure to edit your commit message, though! + ### Release Procedure #### Major Release diff --git a/README.md b/README.md index edfc9220f..3a5a9e237 100644 --- a/README.md +++ b/README.md @@ -9,7 +9,7 @@ Or [Blockchain](), for shor [![version](https://img.shields.io/github/tag/tendermint/tendermint.svg)](https://github.com/tendermint/tendermint/releases/latest) [![API Reference](https://camo.githubusercontent.com/915b7be44ada53c290eb157634330494ebe3e30a/68747470733a2f2f676f646f632e6f72672f6769746875622e636f6d2f676f6c616e672f6764646f3f7374617475732e737667)](https://godoc.org/github.com/tendermint/tendermint) [![Go version](https://img.shields.io/badge/go-1.13-blue.svg)](https://github.com/moovweb/gvm) -[![Discord](https://img.shields.io/discord/669268347736686612.svg)](https://discord.gg/AzefAFd) +[![Discord](https://img.shields.io/discord/669268347736686612.svg)](https://discord.gg/AzefAFd) [![license](https://img.shields.io/github/license/tendermint/tendermint.svg)](https://github.com/tendermint/tendermint/blob/master/LICENSE) [![](https://tokei.rs/b1/github/tendermint/tendermint?category=lines)](https://github.com/tendermint/tendermint) @@ -68,17 +68,16 @@ See the [install instructions](/docs/introduction/install.md) ## Contributing -Please abide by the [Code of Conduct](CODE_OF_CONDUCT.md) in all interactions, -and the [contributing guidelines](CONTRIBUTING.md) when submitting code. +Please abide by the [Code of Conduct](CODE_OF_CONDUCT.md) in all interactions. -Join the larger community on the [forum](https://forum.cosmos.network/) and the [chat](https://riot.im/app/#/room/#tendermint:matrix.org). +Before contributing to the project, please take a look at the [contributing guidelines](CONTRIBUTING.md) +and the [style guide](STYLE_GUIDE.md). -To learn more about the structure of the software, watch the [Developer -Sessions](/docs/DEV_SESSIONS.md) and read some [Architectural Decision -Records](https://github.com/tendermint/tendermint/tree/master/docs/architecture). +To get more active, Join the wider community at [Discord](https://discord.gg/AzefAFd) or jump onto the [Forum](https://forum.cosmos.network/). -Learn more by reading the code and comparing it to the -[specification](https://github.com/tendermint/spec). +Learn more by reading the code and the +[specifications](https://github.com/tendermint/spec) or watch the [Developer Sessions](/docs/DEV_SESSIONS.md) and read up on the +[Architectural Decision Records](https://github.com/tendermint/tendermint/tree/master/docs/architecture). ## Versioning diff --git a/STYLE_GUIDE.md b/STYLE_GUIDE.md new file mode 100644 index 000000000..6d0deda6d --- /dev/null +++ b/STYLE_GUIDE.md @@ -0,0 +1,159 @@ +# Go Coding Style Guide + +In order to keep our code looking good with lots of programmers working on it, it helps to have a "style guide", so all +the code generally looks quite similar. This doesn't mean there is only one "right way" to write code, or even that this +standard is better than your style. But if we agree to a number of stylistic practices, it makes it much easier to read +and modify new code. Please feel free to make suggestions if there's something you would like to add or modify. + +We expect all contributors to be familiar with [Effective Go](https://golang.org/doc/effective_go.html) +(and it's recommended reading for all Go programmers anyways). Additionally, we generally agree with the suggestions + in [Uber's style guide](https://github.com/uber-go/guide/blob/master/style.md) and use that as a starting point. + + +## Code Structure + +Perhaps more key for code readability than good commenting is having the right structure. As a rule of thumb, try to write +in a logical order of importance, taking a little time to think how to order and divide the code such that someone could +scroll down and understand the functionality of it just as well as you do. A loose example of such order would be: +* Constants, global and package-level variables +* Main Struct +* Options (only if they are seen as critical to the struct else they should be placed in another file) +* Initialization / Start and stop of the service +* Msgs/Events +* Public Functions (In order of most important) +* Private/helper functions +* Auxiliary structs and function (can also be above private functions or in a separate file) + +## General + + * Use `gofmt` (or `goimport`) to format all code upon saving it. (If you use VIM, check out vim-go). + * Use a linter (see below) and generally try to keep the linter happy (where it makes sense). + * Think about documentation, and try to leave godoc comments, when it will help new developers. + * Every package should have a high level doc.go file to describe the purpose of that package, its main functions, and any other relevant information. + * `TODO` should not be used. If important enough should be recorded as an issue. + * `BUG` / `FIXME` should be used sparingly to guide future developers on some of the vulnerabilities of the code. + * `XXX` can be used in work-in-progress (prefixed with "WIP:" on github) branches but they must be removed before approving a PR. + * Applications (e.g. clis/servers) *should* panic on unexpected unrecoverable errors and print a stack trace. + +## Comments + + * Use a space after comment deliminter (ex. `// your comment`). + * Many comments are not sentences. These should begin with a lower case letter and end without a period. + * Conversely, sentences in comments should be sentenced-cased and end with a period. + +## Linters + +These must be applied to all (Go) repos. + + * [shellcheck](https://github.com/koalaman/shellcheck) + * [golangci-lint](https://github.com/golangci/golangci-lint) (covers all important linters) + - See the `.golangci.yml` file in each repo for linter configuration. + +## Various + + * Reserve "Save" and "Load" for long-running persistence operations. When parsing bytes, use "Encode" or "Decode". + * Maintain consistency across the codebase. + * Functions that return functions should have the suffix `Fn` + * Names should not [stutter](https://blog.golang.org/package-names). For example, a struct generally shouldn’t have + a field named after itself; e.g., this shouldn't occur: +``` golang +type middleware struct { + middleware Middleware +} +``` + * In comments, use "iff" to mean, "if and only if". + * Product names are capitalized, like "Tendermint", "Basecoin", "Protobuf", etc except in command lines: `tendermint --help` + * Acronyms are all capitalized, like "RPC", "gRPC", "API". "MyID", rather than "MyId". + * Prefer errors.New() instead of fmt.Errorf() unless you're actually using the format feature with arguments. + +## Importing Libraries + +Sometimes it's necessary to rename libraries to avoid naming collisions or ambiguity. + + * Use [goimports](https://godoc.org/golang.org/x/tools/cmd/goimports) + * Separate imports into blocks - one for the standard lib, one for external libs and one for application libs. + * Here are some common library labels for consistency: + - dbm "github.com/tendermint/tm-db" + - tmcmd "github.com/tendermint/tendermint/cmd/tendermint/commands" + - tmcfg "github.com/tendermint/tendermint/config/tendermint" + - tmtypes "github.com/tendermint/tendermint/types" + * Never use anonymous imports (the `.`), for example, `tmlibs/common` or anything else. + * When importing a pkg from the `tendermint/libs` directory, prefix the pkg alias with tm. + - tmbits "github.com/tendermint/tendermint/libs/bits" + * tip: Use the `_` library import to import a library for initialization effects (side effects) + +## Dependencies + + * Dependencies should be pinned by a release tag, or specific commit, to avoid breaking `go get` when external dependencies are updated. + * Refer to the [contributing](CONTRIBUTING.md) document for more details + +## Testing + + * The first rule of testing is: we add tests to our code + * The second rule of testing is: we add tests to our code + * For Golang testing: + * Make use of table driven testing where possible and not-cumbersome + - [Inspiration](https://dave.cheney.net/2013/06/09/writing-table-driven-tests-in-go) + * Make use of [assert](https://godoc.org/github.com/stretchr/testify/assert) and [require](https://godoc.org/github.com/stretchr/testify/require) + * When using mocks, it is recommended to use Testify [mock] (https://pkg.go.dev/github.com/stretchr/testify/mock + ) along with [Mockery](https://github.com/vektra/mockery) for autogeneration + +## Errors + + * Ensure that errors are concise, clear and traceable. + * Use stdlib errors package. + * For wrapping errors, use `fmt.Errorf()` with `%w`. + * Panic is appropriate when an internal invariant of a system is broken, while all other cases (in particular, + incorrect or invalid usage) should return errors. + +## Config + + * Currently the TOML filetype is being used for config files + * A good practice is to store per-user config files under `~/.[yourAppName]/config.toml` + +## CLI + + * When implementing a CLI use [Cobra](https://github.com/spf13/cobra) and [Viper](https://github.com/spf13/viper). + * Helper messages for commands and flags must be all lowercase. + * Instead of using pointer flags (eg. `FlagSet().StringVar`) use Viper to retrieve flag values (eg. `viper.GetString`) + - The flag key used when setting and getting the flag should always be stored in a + variable taking the form `FlagXxx` or `flagXxx`. + - Flag short variable descriptions should always start with a lower case character as to remain consistent with + the description provided in the default `--help` flag. + +## Version + + * Every repo should have a version/version.go file that mimics the Tendermint Core repo + * We read the value of the constant version in our build scripts and hence it has to be a string + +## Non-Go Code + + * All non-Go code (`*.proto`, `Makefile`, `*.sh`), where there is no common + agreement on style, should be formatted according to + [EditorConfig](http://editorconfig.org/) config: + + ``` + # top-most EditorConfig file + root = true + + # Unix-style newlines with a newline ending every file + [*] + charset = utf-8 + end_of_line = lf + insert_final_newline = true + trim_trailing_whitespace = true + + [Makefile] + indent_style = tab + + [*.sh] + indent_style = tab + + [*.proto] + indent_style = space + indent_size = 2 + ``` + + Make sure the file above (`.editorconfig`) are in the root directory of your + repo and you have a [plugin for your + editor](http://editorconfig.org/#download) installed.