From 1e6fc74d4fcf7aba92d0b0a8dba15106eecbd1ae Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Fri, 6 Oct 2017 11:23:49 +0100 Subject: [PATCH] Add Code Style and some dev docs (#286) --- CODE_STYLE.md | 63 +++++++++++++++++++++++++++++++++++++++++++++++++ CONTRIBUTING.md | 13 ++++++++++ INSTALL.md | 8 +++---- 3 files changed, 80 insertions(+), 4 deletions(-) create mode 100644 CODE_STYLE.md diff --git a/CODE_STYLE.md b/CODE_STYLE.md new file mode 100644 index 000000000..0e0a043ad --- /dev/null +++ b/CODE_STYLE.md @@ -0,0 +1,63 @@ +# Code Style + +We follow the standard Go style using gofmt, but with a few extra +considerations. + +## Linters + +We use `gometalinter` to run a number of linters, the exact list can be found +in [linter.json](linter.json). Some of these are slow and expensive to run, but +a subset can be found in [linter-fast.json](linter-fast.json) that run quickly +enough to be run as part of an IDE. + +For rare cases where a linter is giving a spurious warning, it can be disabled +for that line or statement using a [comment directive](https://github.com/alecthomas/gometalinter#comment-directives), e.g. +`// nolint: gocyclo`. This should be used sparingly and only when its clear +that the lint warning is spurious. + + +## HTTP Error Handling + +Unfortunately, converting errors into HTTP responses with the correct status +code and message can be done in a number of ways in golang: + +1. Having functions return `JSONResponse` directly, which can then either set + it to an error response or a `200 OK`. +2. Have the HTTP handler try and cast error values to types that are handled + differently. +3. Have the HTTP handler call functions whose errors can only be interpreted + one way, for example if a `validate(...)` call returns an error then handler + knows to respond with a `400 Bad Request`. + +We attempt to always use option #3, as it more naturally fits with the way that +golang generally does error handling. In particular, option #1 effectively +requires reinventing a new error handling scheme just for HTTP handlers. + + +## Line length + +We strive for a line length of roughly 80 characters, though less than 100 is +acceptable if necessary. Longer lines are fine if there is nothing of interest +after the first 80-100 characters (e.g. long string literals). + + +## TODOs and FIXMEs + +The majority of TODOs and FIXMEs should have an associated tracking issue on +github. These can be added just before merging of the PR to master, and the +issue number should be added to the comment, e.g. `// TODO(#324): ...` + + +## Visual Studio Code + +If you use VSCode then the following is an example of a workspace setting that +sets up linting correctly: + +```json +{ + "go.gopath": "${workspaceRoot}:${workspaceRoot}/vendor", + "go.lintOnSave": "workspace", + "go.lintTool": "gometalinter", + "go.lintFlags": ["--config=linter-fast.json", "--concurrency=5"] +} +``` diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index e5e4ca8ff..00761f58e 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -3,6 +3,19 @@ Everyone is welcome to contribute to Dendrite! We aim to make it as easy as possible to get started. +Please ensure that you sign off your contributions! See [Sign Off](#sign-off) +section below. + +## Getting up and running + +See [INSTALL.md](INSTALL.md) for instructions on setting up a running dev +instance of dendrite, and [CODE_STYLE.md](CODE_STYLE.md) for the code style +guide. + +We use `gb` for managing our dependencies, so `gb build` and `gb test` is how +to build dendrite and run the unit tests respectively. + + ## Picking Things To Do If you're new then feel free to pick up an issue labelled [easy](https://github.com/matrix-org/dendrite/labels/easy). diff --git a/INSTALL.md b/INSTALL.md index f562cd492..1d002c5fd 100644 --- a/INSTALL.md +++ b/INSTALL.md @@ -5,7 +5,7 @@ Dendrite can be run in one of two configurations: * A cluster of individual components, dealing with different aspects of the Matrix protocol (see [WIRING.md](./WIRING.md)). Components communicate with one another via [Apache Kafka](https://kafka.apache.org). - + * A monolith server, in which all components run in the same process. In this configuration, Kafka can be replaced with an in-process implementation called [naffka](https://github.com/matrix-org/naffka). @@ -17,7 +17,7 @@ Dendrite can be run in one of two configurations: - For Kafka (optional if using the monolith server): - Unix-based system (https://kafka.apache.org/documentation/#os) - JDK 1.8+ / OpenJDK 1.8+ - - Apache Kafka 0.10.2+ (see https://github.com/matrix-org/dendrite/blob/master/travis-install-kafka.sh for up-to-date version numbers) + - Apache Kafka 0.10.2+ (see [travis-install-kafka.sh](travis-install-kafka.sh) for up-to-date version numbers) ## Setting up a development environment @@ -34,7 +34,7 @@ go get github.com/constabulary/gb/... gb build ``` -If using Kafka, install and start it: +If using Kafka, install and start it (c.f. [travis-install-kafka.sh](travis-install-kafka.sh)): ```bash MIRROR=http://apache.mirror.anlx.net/kafka/0.10.2.0/kafka_2.11-0.10.2.0.tgz @@ -108,7 +108,7 @@ The following contains scripts which will run all the required processes in orde ``` - /media +---------------------------+ + /media +---------------------------+ +----------->+------------->| dendrite-media-api-server | ^ ^ +---------------------------+ | | :7774