From 94073ce71d28f4b0df020081caf5b0eb0381189f Mon Sep 17 00:00:00 2001 From: Manuel Mendez Date: Mon, 6 Dec 2021 11:21:13 -0500 Subject: [PATCH] Style: Enforce maximum of 2 import groups for go files There isn't a whole lot of direction from the go project or tools for this. A quick google search did turn up 2 projects that also document 2 groups, both [Uber] and [CockroachDB]. We do have both goimports and gofumpt splitting stdlib vs others out if they are mixed. But they don't do any futher kind of grouping. So in the absence of concrete handling lets just settle on 2 because its easy to enforce. The script below deletes newlines within `import ( ... )` blocks and then runs gofumpt which will split and sort 2 at most 2 groups. [Uber]: https://github.com/uber-go/guide/blob/22f61e283ac2f75ba34b62f565581cc70e35fb7a/style.md#import-group-ordering [CockroachDB]: https://web.archive.org/web/20210509102622/https://wiki.crdb.io/wiki/spaces/CRDB/pages/181371303/Go+Golang+coding+guidelines#Import-Group-Ordering Signed-off-by: Manuel Mendez --- .github/workflows/ci.yaml | 3 ++- CONTRIBUTING.md | 6 ++++++ ci-checks.sh | 4 ++++ 3 files changed, 12 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 5fc701aa8..026b09df0 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -51,7 +51,8 @@ jobs: nix_path: nixpkgs=channel:nixos-unstable - name: Fetch Nix Packages run: nix-shell --run 'true' - - run: ./ci-checks.sh + - run: make bin/gofumpt + - run: PATH=$PWD/bin/:$PATH ./ci-checks.sh validation: runs-on: ubuntu-latest needs: diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 1d5da05ed..76d7d1411 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -98,6 +98,12 @@ One must support their proposed changes with unit tests. As you submit a pull request(PR) the CI generates a code coverage report. It will help you identify parts of the code that are not yet covered in unit tests. +#### Go + +##### Import Groups + +There should be two groups of import blocks, one for stdlib and the other for everything else. + ## Understanding code structure This is a nonexhaustive list important packages that happen to cover most of the code base. diff --git a/ci-checks.sh b/ci-checks.sh index ed5dde6f5..c793c1123 100755 --- a/ci-checks.sh +++ b/ci-checks.sh @@ -28,6 +28,10 @@ if ! nixfmt shell.nix; then failed=1 fi +if ! git ls-files '*.go' | xargs -I% sh -c 'sed -i "/^import (/,/^)/ { /^\s*$/ d }" % && gofumpt -w -s %'; then + failed=1 +fi + if ! git diff | (! grep .); then failed=1 fi