This document is a collection of team preferences and agreements. It can be updated at any time (PRs welcome) when opinions change. The advice below will not apply to all situations. Please use your best judgement and ignore these preferences when it makes sense.
Each developer is responsible for their feature, from the first commit to testing it in production. The developer is responsible for adding tests, reaching out for PR reviews in Slack, testing in staging, and testing in production after it is released.
Leaving undeployed code on the main branch introduces risk. The developer who owns a commit is the best person to debug any issues that may occur in production and the best time to debug code is as close to when it was written as possible.
One common exception to this rule is letting changes soak in stage2. In this case, the developer should post in Slack to ensure no one else on the team accidentally deploys the changes to production.
The larger a PR, the less likely someone will catch all nuances of it. Prefer small PRs over large PRs. If a PR is longer than 1000 lines changed, it should be broken up.
As a team, we strive to leave a code base better than it was before we touched it. Pull requests should improve the test coverage when possible. A feature being difficult to unit test is a code smell. If testing is difficult, the developer should investigate restructuring his code to make it easier to test.
Developers should include instructions to test the PR. Did you use curl or grpcurl? Assume a dedicated reviewer will pull the branch and test it locally. How would they do that? Try to provide any related context the reviewers might need to understand the PR.
Large features should have a draft PR to get feedback as early as possible. When posting the draft PR, include a list of outstanding todos to help the reviewers.
For small changes, especially typos, we can reduce the feedback loop by using Github's suggest feature.
Unless the reviewer can link to the style guide rule being violated, avoid personal preferences in PR feedback. Reviewers need to accept other developer's solutions/styles and should never block a PR over nit-picky feedback. Unless you can point to a specific rule in the style guide, stick avoid style comments.
Don't mix tabs and spaces for indentation. Update your editor to remove trailing whitespace on save.
Imports be broken up into three sections separated by new lines for standart library imports, DO imports, and third-party imports. The team has decided that the order for imports should be as follows.
import (
stdlib imports
third-party imports
project imports
)
All required styles should be enforced via a CI or linter. Developers should setup their IDE to lint on save.
Developers should write table tests for new code. Table tests provide the following benefits.
- Setup of the test is separated from the test cases making the test conditions more obvious to developers.
- Updating the tests is easier when the code structure changes. If a new variable is added to a method, the tests are changed in one spot instead of multiple funcs.
- The simplicity of adding new test cases makes it easier for developers to include more edge cases in their unit tests.
The tests should use the same variable names in tests. For example, the array of tests cases variable should be named tests. This will improve navigating and modifying tests in code.
tests := []struct {
name string
setup func(sqlmock.Sqlmock)
expected struct{}
err error
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
tt := tt
t.Parallel() // if applicable
})
}
https://pkg.go.dev/github.com/stretchr/testify/require
When setting up the test, use testify/require to assert certain conditions such as require.NoError(t, err) or require.NotNil(t, struct). When a require assertion fails, the tests will immediately exit.
https://pkg.go.dev/github.com/stretchr/testify/assert
Use testify/assert to have more specific assertions. There are assert methods for comparing JSON, that two arrays have the same elements, and many more.
Increase test througput by enabling table tests to run in parallel.
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
tt := tt // capture the pointer to the test case within this scope
t.Parallel() // signal that this test is ready to be run in parallel with other tests
})
}
First, functions that need to make http requests should share the same http.Client struct. http.Client is goroutine safe and will reuse connections if possible. Create your http.Client in your main.go, add your service's user agent to it, and pass it as a dependency to all of your packages.
When testing code that makes an http request, override the Transport field of your http.Client with a custom RoundTripper. This allows you to mock responses without having to run a httptest.Server.
https://github.com/golang-standards/project-layout
Put the binaries in the cmd directory under the binary's name. By default go build will compile main.go into a binary with the same name as the folder it is in. It is incredibly likely that your code base will have multiple binaries, such as a ctl or docc job.
cmd/<server-binary>/main.go
/<command-line-tool>/main.go
/<job-binary>/main.go
A code smell is passing your config struct to your dependencies. Ensure that packages can't import config types by putting it in the same directory/package as your main.go
build - Build the binary on the local system.
clean - Remove the build directory and any other build artifacts.
coverage - Run go test with coverage enabled.
docker-build - Build the binary in the same docker container it. This is used by docker-run an docker-push.
docker-deps - Start the docker-compose dependencies in the background
docker-push - Build the docker file and push the image to the docker registry.
docker-run - Build and execute the docker binary. This is the equivalent of `make docker-build && docker-compose up`
docker-stop - The equivalent of docker-compose stop.
lint - Execute gofmt and Go's staticcheck tool.
gen - Run Go generators. This should rebuild any mocks.
run - Equivalent of `go run`.
test - Execute tests in the current directory and all subdirectories.