From ffd774372249ce399edcd2b4b9ea5ebb5739c03d Mon Sep 17 00:00:00 2001 From: Dmitry K Date: Thu, 1 Sep 2022 16:59:05 -0700 Subject: [PATCH 01/18] add linters --- .github/workflows/main.yml | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index fbe7ff9b1..968c10929 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -16,7 +16,7 @@ on: env: # Even though we can test against multiple versions, this one is considered a target version. - TARGET_GOLANG_VERSION: "1.18" + TARGET_GOLANG_VERSION: "1.19" PROTOC_VERSION: "3.19.4" jobs: @@ -24,7 +24,7 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - go: ["1.18"] # As we are relying on generics, we can't go lower than 1.18. + go: ["1.18", "1.19"] # As we are relying on generics, we can't go lower than 1.18. fail-fast: false name: Go ${{ matrix.go }} test steps: @@ -58,7 +58,10 @@ jobs: uses: guyarb/golang-test-annotations@v0.5.1 with: test-results: test_results.json - + - name: Run static check + # Only run if the test failed on target version to avoid duplicated annotations on GitHub. + if: ${{ always() && env.TARGET_GOLANG_VERSION == matrix.go }} + uses: reviewdog/action-staticcheck@v1 # TODO(@okdas): move coverage to a separate job - name: create coverage report if: ${{ env.TARGET_GOLANG_VERSION == matrix.go }} From 6e20b6454b027a36bbfdcd84d66507da6dc6be6f Mon Sep 17 00:00:00 2001 From: Dmitry K Date: Fri, 2 Sep 2022 16:05:39 -0700 Subject: [PATCH 02/18] use separate workflow for linters --- .github/workflows/{pr.yml => ggshield.yml} | 6 +++-- .github/workflows/linters.yml | 26 ++++++++++++++++++++++ .github/workflows/main.yml | 11 +++++++-- 3 files changed, 39 insertions(+), 4 deletions(-) rename .github/workflows/{pr.yml => ggshield.yml} (85%) create mode 100644 .github/workflows/linters.yml diff --git a/.github/workflows/pr.yml b/.github/workflows/ggshield.yml similarity index 85% rename from .github/workflows/pr.yml rename to .github/workflows/ggshield.yml index 1fedcc015..20e7dffc5 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/ggshield.yml @@ -1,6 +1,8 @@ name: GitGuardian scan -on: [push, pull_request] +on: + workflow_dispatch: + push: jobs: scanning: @@ -16,6 +18,6 @@ jobs: env: GITHUB_PUSH_BEFORE_SHA: ${{ github.event.before }} GITHUB_PUSH_BASE_SHA: ${{ github.event.base }} - GITHUB_PULL_BASE_SHA: ${{ github.event.pull_request.base.sha }} + GITHUB_PULL_BASE_SHA: ${{ github.event.pull_request.base.sha }} GITHUB_DEFAULT_BRANCH: ${{ github.event.repository.default_branch }} GITGUARDIAN_API_KEY: ${{ secrets.GITGUARDIAN_API_KEY }} diff --git a/.github/workflows/linters.yml b/.github/workflows/linters.yml new file mode 100644 index 000000000..7dd9073fd --- /dev/null +++ b/.github/workflows/linters.yml @@ -0,0 +1,26 @@ +name: Linters, static checks and code coverage + +on: + workflow_dispatch: + # push: + # branches: [main, dk-add-linters] + # paths-ignore: + # - "docs/**" + # - "**.md" + # pull_request: + # paths-ignore: + # - "docs/**" + # - "**.md" + +jobs: + run-linters: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + # - name: Run static check + # uses: dominikh/staticcheck-action@v1.2.0 + # - name: Run static check + # uses: reviewdog/action-staticcheck@v1 + # with: + # # TODO: pick filter https://github.com/reviewdog/reviewdog#filter-mode + # filter_mode: nofilter diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 968c10929..c01666d14 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -62,12 +62,19 @@ jobs: # Only run if the test failed on target version to avoid duplicated annotations on GitHub. if: ${{ always() && env.TARGET_GOLANG_VERSION == matrix.go }} uses: reviewdog/action-staticcheck@v1 + with: + # TODO: pick filter https://github.com/reviewdog/reviewdog#filter-mode + filter_mode: nofilter + - name: Run golangci-lint + # Only run if the test failed on target version to avoid duplicated annotations on GitHub. + if: ${{ always() && env.TARGET_GOLANG_VERSION == matrix.go }} + uses: golangci/golangci-lint-action@v3 # TODO(@okdas): move coverage to a separate job - name: create coverage report - if: ${{ env.TARGET_GOLANG_VERSION == matrix.go }} + if: ${{ always() && env.TARGET_GOLANG_VERSION == matrix.go }} run: make test_all_with_coverage - name: Upload coverage to Codecov - if: ${{ env.TARGET_GOLANG_VERSION == matrix.go }} + if: ${{ always() && env.TARGET_GOLANG_VERSION == matrix.go }} uses: codecov/codecov-action@v3 # TODO(@okdas): reuse artifacts built by the previous job instead From 27684519163a2fcbe26957b2d6d446a459afcb72 Mon Sep 17 00:00:00 2001 From: Dmitry K Date: Fri, 2 Sep 2022 17:35:20 -0700 Subject: [PATCH 03/18] ci wip --- .github/workflows/main.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index c01666d14..73817a8e8 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -62,9 +62,9 @@ jobs: # Only run if the test failed on target version to avoid duplicated annotations on GitHub. if: ${{ always() && env.TARGET_GOLANG_VERSION == matrix.go }} uses: reviewdog/action-staticcheck@v1 - with: - # TODO: pick filter https://github.com/reviewdog/reviewdog#filter-mode - filter_mode: nofilter + # with: + # # TODO: pick filter https://github.com/reviewdog/reviewdog#filter-mode + # filter_mode: nofilter - name: Run golangci-lint # Only run if the test failed on target version to avoid duplicated annotations on GitHub. if: ${{ always() && env.TARGET_GOLANG_VERSION == matrix.go }} From 5b1b7bdb1a466236e35ab33230964e74eb12bbb0 Mon Sep 17 00:00:00 2001 From: Dmitry K Date: Thu, 8 Sep 2022 17:21:03 -0700 Subject: [PATCH 04/18] use 1.19 only, run reviewdog against all file --- .github/workflows/main.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 73817a8e8..90d222c16 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -24,7 +24,7 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - go: ["1.18", "1.19"] # As we are relying on generics, we can't go lower than 1.18. + go: ["1.19"] # As we are relying on generics, we can't go lower than 1.18. fail-fast: false name: Go ${{ matrix.go }} test steps: @@ -62,9 +62,9 @@ jobs: # Only run if the test failed on target version to avoid duplicated annotations on GitHub. if: ${{ always() && env.TARGET_GOLANG_VERSION == matrix.go }} uses: reviewdog/action-staticcheck@v1 - # with: - # # TODO: pick filter https://github.com/reviewdog/reviewdog#filter-mode - # filter_mode: nofilter + with: + # TODO(@okdas): pick filter https://github.com/reviewdog/reviewdog#filter-mode + filter_mode: nofilter - name: Run golangci-lint # Only run if the test failed on target version to avoid duplicated annotations on GitHub. if: ${{ always() && env.TARGET_GOLANG_VERSION == matrix.go }} From 91ee954ed3ba8ce043e033e0733ed1aab0123817 Mon Sep 17 00:00:00 2001 From: Dmitry K Date: Tue, 20 Sep 2022 18:35:56 -0700 Subject: [PATCH 05/18] some docs and version changes --- .github/workflows/linters.yml | 26 -------------------------- .github/workflows/main.yml | 3 +-- Makefile | 7 +++++++ build/Dockerfile.debian.dev | 6 +++--- build/Dockerfile.debian.prod | 4 ++-- docs/build/automatic-builds.md | 14 -------------- docs/development/README.md | 13 +++++++++++++ docs/{build => releases}/README.md | 18 +++++++++++++++--- 8 files changed, 41 insertions(+), 50 deletions(-) delete mode 100644 .github/workflows/linters.yml delete mode 100644 docs/build/automatic-builds.md rename docs/{build => releases}/README.md (64%) diff --git a/.github/workflows/linters.yml b/.github/workflows/linters.yml deleted file mode 100644 index 7dd9073fd..000000000 --- a/.github/workflows/linters.yml +++ /dev/null @@ -1,26 +0,0 @@ -name: Linters, static checks and code coverage - -on: - workflow_dispatch: - # push: - # branches: [main, dk-add-linters] - # paths-ignore: - # - "docs/**" - # - "**.md" - # pull_request: - # paths-ignore: - # - "docs/**" - # - "**.md" - -jobs: - run-linters: - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v3 - # - name: Run static check - # uses: dominikh/staticcheck-action@v1.2.0 - # - name: Run static check - # uses: reviewdog/action-staticcheck@v1 - # with: - # # TODO: pick filter https://github.com/reviewdog/reviewdog#filter-mode - # filter_mode: nofilter diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 90d222c16..65eee3ca3 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -63,13 +63,12 @@ jobs: if: ${{ always() && env.TARGET_GOLANG_VERSION == matrix.go }} uses: reviewdog/action-staticcheck@v1 with: - # TODO(@okdas): pick filter https://github.com/reviewdog/reviewdog#filter-mode + # TODO(@okdas): change filter after bug bash to only show annotations on files changed by PR (currently shows all changes) https://github.com/reviewdog/reviewdog#filter-mode filter_mode: nofilter - name: Run golangci-lint # Only run if the test failed on target version to avoid duplicated annotations on GitHub. if: ${{ always() && env.TARGET_GOLANG_VERSION == matrix.go }} uses: golangci/golangci-lint-action@v3 - # TODO(@okdas): move coverage to a separate job - name: create coverage report if: ${{ always() && env.TARGET_GOLANG_VERSION == matrix.go }} run: make test_all_with_coverage diff --git a/Makefile b/Makefile index 86b768812..364d18359 100644 --- a/Makefile +++ b/Makefile @@ -317,6 +317,13 @@ test_p2p: test_p2p_addrbook: go test -run AddrBook -v -count=1 ./p2p/... +.PHONY: lint +## Run all P2P addr book related tests +lint: + staticcheck -f stylish ./... || true + golangci-lint run ./... || true + echo "return code is always successfull, as we want to see all the linting errors" + .PHONY: benchmark_sortition ## Benchmark the Sortition library benchmark_sortition: diff --git a/build/Dockerfile.debian.dev b/build/Dockerfile.debian.dev index 5f0ab8bbe..8e410b670 100644 --- a/build/Dockerfile.debian.dev +++ b/build/Dockerfile.debian.dev @@ -1,7 +1,7 @@ # Purpose of this container image is to ship pocket binary with additional # tools such as dlv, curl, etc. -ARG TARGET_GOLANG_VERSION=1.18 +ARG TARGET_GOLANG_VERSION=1.19 FROM golang:${TARGET_GOLANG_VERSION}-bullseye AS builder @@ -33,7 +33,7 @@ RUN set -eux; \ unzip -q protoc.zip -d /usr/local/; \ protoc --version -# dlv +# dlv RUN go install github.com/go-delve/delve/cmd/dlv@latest; \ dlv version @@ -52,4 +52,4 @@ RUN go get -d -v ./app/pocket RUN go build -o /usr/local/bin/pocket ./app/pocket RUN go build -o /usr/local/bin/client ./app/client -CMD ["/usr/local/bin/pocket"] \ No newline at end of file +CMD ["/usr/local/bin/pocket"] diff --git a/build/Dockerfile.debian.prod b/build/Dockerfile.debian.prod index 6d65b44ac..a95ae3cb1 100644 --- a/build/Dockerfile.debian.prod +++ b/build/Dockerfile.debian.prod @@ -1,6 +1,6 @@ # Purpose of this container image is to ship pocket binary with minimal dependencies. -ARG TARGET_GOLANG_VERSION=1.18 +ARG TARGET_GOLANG_VERSION=1.19 FROM golang:${TARGET_GOLANG_VERSION}-bullseye AS builder @@ -53,4 +53,4 @@ RUN apt-get update -qq && \ COPY --from=builder /go/bin/pocket /usr/local/bin/pocket -CMD ["/usr/local/bin/pocket"] \ No newline at end of file +CMD ["/usr/local/bin/pocket"] diff --git a/docs/build/automatic-builds.md b/docs/build/automatic-builds.md deleted file mode 100644 index 2ffe52af4..000000000 --- a/docs/build/automatic-builds.md +++ /dev/null @@ -1,14 +0,0 @@ -# Automatic Builds - -We have automation that creates new container images for the Pocket Network v1 Node. - -## Tags - -Code built from default branch (i.e. `main`) is tagged as `latest`. - -Code built from commits in Pull Requests, is tagged as `pr-`, as well as `sha-<7 digit sha>`. - - -### Extended images with additional tooling - -We also supply an extended image with tooling for each container tag to help you troubleshoot or investigate issues. The extended image is called `-dev`. For example, `latest-dev`, or `pr-123-dev`. diff --git a/docs/development/README.md b/docs/development/README.md index 1cc00113b..98fd1efec 100644 --- a/docs/development/README.md +++ b/docs/development/README.md @@ -10,6 +10,7 @@ Please note that this repository is under very active development and breaking c - [Running Unit Tests](#running-unit-tests) - [Running LocalNet](#running-localnet) - [Code Organization](#code-organization) + - [Linters](#linters) ## LFG - Development @@ -156,3 +157,15 @@ Pocket ├── utility # Implementation of the Utility module ├── Makefile # [to-be-deleted] The source of targets used to develop, build and test ``` + +### Linters + +Our CI automation runs some linters and static checks to ensure code quality. You can run them locally with: + +```bash +make lint +``` + +You might need to install some of the linters locally, here is a list of them. +* `staticcheck`. Can be installed via `go install honnef.co/go/tools/cmd/staticcheck@latest` (assuming `$GOPATH/bin` is in your `$PATH`). +* `golangci-lint`. Installation instructions provided [here](https://golangci-lint.run/usage/install/#local-installation). diff --git a/docs/build/README.md b/docs/releases/README.md similarity index 64% rename from docs/build/README.md rename to docs/releases/README.md index 885fb322a..392eee98f 100644 --- a/docs/build/README.md +++ b/docs/releases/README.md @@ -12,13 +12,25 @@ Automatic development / test / production builds are still a work in progress, b - `+dirty` for uncommited changes - `-version` flag that can be injected or defaults to `UNKNOWN` -- `branch_name` and a shortened `commit_hash` shold be included +- `branch_name` and a shortened `commit_hash` should be included For example, `X.Y.Z[-`, as well as `sha-<7 digit sha>`. + +Once we have a release, we will tag the release with the version number (e.g. `v0.0.1-alpha.pre.1`). + + +### Extended images with additional tooling + +We also supply an extended image with tooling for each container tag to help you troubleshoot or investigate issues. The extended image is called `-dev`. For example, `latest-dev`, or `pr-123-dev`. -* Docker images as a part of [automatic builds](automatic-builds.md) ## [TODO] Magefile build system From 8a5f48b050a75a6c05c71f060f50a285a99636dd Mon Sep 17 00:00:00 2001 From: Dmitry K Date: Fri, 23 Sep 2022 16:38:47 -0700 Subject: [PATCH 06/18] golangci-lint covers staticcheck --- .github/workflows/main.yml | 14 ++++----- .golangci.yml | 59 +++++++++++++++++++++++++++++++++++++ Makefile | 4 +-- go.mod | 1 + go.sum | 2 ++ misc/linting-rules/tests.go | 19 ++++++++++++ 6 files changed, 89 insertions(+), 10 deletions(-) create mode 100644 .golangci.yml create mode 100644 misc/linting-rules/tests.go diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 65eee3ca3..50b9cc0a9 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -58,13 +58,13 @@ jobs: uses: guyarb/golang-test-annotations@v0.5.1 with: test-results: test_results.json - - name: Run static check - # Only run if the test failed on target version to avoid duplicated annotations on GitHub. - if: ${{ always() && env.TARGET_GOLANG_VERSION == matrix.go }} - uses: reviewdog/action-staticcheck@v1 - with: - # TODO(@okdas): change filter after bug bash to only show annotations on files changed by PR (currently shows all changes) https://github.com/reviewdog/reviewdog#filter-mode - filter_mode: nofilter + # - name: Run static check + # # Only run if the test failed on target version to avoid duplicated annotations on GitHub. + # if: ${{ always() && env.TARGET_GOLANG_VERSION == matrix.go }} + # uses: reviewdog/action-staticcheck@v1 + # with: + # # TODO(@okdas): change filter after bug bash to only show annotations on files changed by PR (currently shows all changes) https://github.com/reviewdog/reviewdog#filter-mode + # filter_mode: nofilter - name: Run golangci-lint # Only run if the test failed on target version to avoid duplicated annotations on GitHub. if: ${{ always() && env.TARGET_GOLANG_VERSION == matrix.go }} diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 000000000..77ef6b3ee --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,59 @@ +linters: + enable: + # TODO: investigate what linters we want to enable (many disabled by default!) + # https://golangci-lint.run/usage/linters/#disabled-by-default + + # Default linters + - errcheck + - gosimple + - govet + - ineffassign + - staticcheck + - typecheck + - unused + + # Additional linters + - gosec + - gofumpt + - misspell + - promlinter + - unparam + + # Gocritic allows to use ruleguard - custom linting rules + - gocritic +linters-settings: + # Gocritic settings + # https://go-critic.com/overview + gocritic: + enabled-checks: + - ruleguard + settings: + ruleguard: + # Enable debug to identify which 'Where' condition was rejected. + # The value of the parameter is the name of a function in a ruleguard file. + # + # When a rule is evaluated: + # If: + # The Match() clause is accepted; and + # One of the conditions in the Where() clause is rejected, + # Then: + # ruleguard prints the specific Where() condition that was rejected. + # + # The flag is passed to the ruleguard 'debug-group' argument. + # Default: "" + # debug: "testEq" + # Determines the behavior when an error occurs while parsing ruleguard files. + # If flag is not set, log error and skip rule files that contain an error. + # If flag is set, the value must be a comma-separated list of error conditions. + # - 'all': fail on all errors. + # - 'import': ruleguard rule imports a package that cannot be found. + # - 'dsl': gorule file does not comply with the ruleguard DSL. + # Default: "" + failOn: dsl + rules: "misc/linting-rules/*.go" +run: + go: "1.19" + skip-dirs: + - misc/linting-rules + build-tags: + - codeanalysis diff --git a/Makefile b/Makefile index 364d18359..02351d4a0 100644 --- a/Makefile +++ b/Makefile @@ -320,9 +320,7 @@ test_p2p_addrbook: .PHONY: lint ## Run all P2P addr book related tests lint: - staticcheck -f stylish ./... || true - golangci-lint run ./... || true - echo "return code is always successfull, as we want to see all the linting errors" + golangci-lint run ./... .PHONY: benchmark_sortition ## Benchmark the Sortition library diff --git a/go.mod b/go.mod index 54191cb69..4b7229fd2 100644 --- a/go.mod +++ b/go.mod @@ -23,6 +23,7 @@ require ( github.com/dgraph-io/badger/v3 v3.2103.2 github.com/jackc/pgconn v1.11.0 github.com/jordanorelli/lexnum v0.0.0-20141216151731-460eeb125754 + github.com/quasilyte/go-ruleguard/dsl v0.3.21 ) require ( diff --git a/go.sum b/go.sum index d064b7f46..23e2e92a7 100644 --- a/go.sum +++ b/go.sum @@ -352,6 +352,8 @@ github.com/prometheus/procfs v0.1.3/go.mod h1:lV6e/gmhEcM9IjHGsFOCxxuZ+z1YqCvr4O github.com/prometheus/procfs v0.6.0/go.mod h1:cz+aTbrPOrUb4q7XlbU9ygM+/jj0fzG6c1xBZuNvfVA= github.com/prometheus/procfs v0.7.3 h1:4jVXhlkAyzOScmCkXBTOLRLTz8EeU+eyjrwB/EPq0VU= github.com/prometheus/procfs v0.7.3/go.mod h1:cz+aTbrPOrUb4q7XlbU9ygM+/jj0fzG6c1xBZuNvfVA= +github.com/quasilyte/go-ruleguard/dsl v0.3.21 h1:vNkC6fC6qMLzCOGbnIHOd5ixUGgTbp3Z4fGnUgULlDA= +github.com/quasilyte/go-ruleguard/dsl v0.3.21/go.mod h1:KeCP03KrjuSO0H1kTuZQCWlQPulDV6YMIXmpQss17rU= github.com/rogpeppe/go-internal v1.3.0/go.mod h1:M8bDsm7K2OlrFYOpmOWEs/qY81heoFRclV5y23lUDJ4= github.com/rogpeppe/go-internal v1.6.1 h1:/FiVV8dS/e+YqF2JvO3yXRFbBLTIuSDkuC7aBOAvL+k= github.com/rogpeppe/go-internal v1.6.1/go.mod h1:xXDCJY+GAPziupqXw64V24skbSoqbTEfhy4qGm1nDQc= diff --git a/misc/linting-rules/tests.go b/misc/linting-rules/tests.go new file mode 100644 index 000000000..ff0ee5453 --- /dev/null +++ b/misc/linting-rules/tests.go @@ -0,0 +1,19 @@ +//nolint // it's not a Go code file +//go:build !codeanalysis +// +build !codeanalysis + +// This file includes our custom linters. +// If you want to add/modify an existing linter, please check out ruleguard documentation: https://github.com/quasilyte/go-ruleguard#documentation + +package gorules + +import ( + "github.com/quasilyte/go-ruleguard/dsl" +) + +// This is a custom linter that checks ensures a use of require.Equal +func EqualInsteadOfTrue(m dsl.Matcher) { + m.Match(`require.True($t, $x == $y, $*args)`). + Suggest(`require.Equal($t, $x, $y, $args)`). + Report(`use require.Equal instead of require.True`) +} From e5779fe5b63023e1f39ea923fab029705b4262fe Mon Sep 17 00:00:00 2001 From: Dmitry Knyazev Date: Mon, 26 Sep 2022 16:14:34 -0700 Subject: [PATCH 07/18] Apply suggestions from code review Co-authored-by: Daniel Olshansky --- Makefile | 2 +- docs/releases/README.md | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Makefile b/Makefile index 02351d4a0..64cea138f 100644 --- a/Makefile +++ b/Makefile @@ -318,7 +318,7 @@ test_p2p_addrbook: go test -run AddrBook -v -count=1 ./p2p/... .PHONY: lint -## Run all P2P addr book related tests +## Run all linters that are triggered by the CI pipeline lint: golangci-lint run ./... diff --git a/docs/releases/README.md b/docs/releases/README.md index 392eee98f..bdd09e175 100644 --- a/docs/releases/README.md +++ b/docs/releases/README.md @@ -20,16 +20,16 @@ For example, `X.Y.Z[-`, as well as `sha-<7 digit sha>`. -Once we have a release, we will tag the release with the version number (e.g. `v0.0.1-alpha.pre.1`). +Once releases are managed, they will be tagged with the version number (e.g. `v0.0.1-alpha.pre.1`). ### Extended images with additional tooling -We also supply an extended image with tooling for each container tag to help you troubleshoot or investigate issues. The extended image is called `-dev`. For example, `latest-dev`, or `pr-123-dev`. +Extended images with additional tooling are built to aid in troubleshoot and debugging. The extended image is formatted as `-dev`. For example, `latest-dev`, or `pr-123-dev`. ## [TODO] Magefile build system From 3253f4f7427631f3be0b5c734180bb09c79cb3aa Mon Sep 17 00:00:00 2001 From: Dmitry K Date: Mon, 26 Sep 2022 16:33:27 -0700 Subject: [PATCH 08/18] turn off docker hub --- .github/workflows/main.yml | 17 ++++++++++------- docs/development/README.md | 1 - 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 50b9cc0a9..f755dd003 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -96,8 +96,11 @@ jobs: id: meta uses: docker/metadata-action@v4 with: + # poktnetwork/pocket-v1 + # ^ the name of the repo on docker hub. For some reason, sometimes docker hub does not authenticate the user + # resulting in an error when trying to push to the repo. + # TODO(@okdas): figure out why that happens, fix or stick to ghcr.io/AWS ECR. images: | - poktnetwork/pocket-v1 ghcr.io/pokt-network/pocket-v1 tags: | type=schedule${{ matrix.imageType == 'dev' && ',suffix=-dev' || '' }} @@ -108,12 +111,12 @@ jobs: type=ref,event=pr${{ matrix.imageType == 'dev' && ',suffix=-dev' || '' }} type=sha${{ matrix.imageType == 'dev' && ',suffix=-dev' || '' }} type=raw,value=latest,enable={{is_default_branch}}${{ matrix.imageType == 'dev' && ',suffix=-dev' || '' }} - # Turned off until we set up docker hub - - name: Login to DockerHub - uses: docker/login-action@v2 - with: - username: ${{ secrets.DOCKERHUB_USERNAME }} - password: ${{ secrets.DOCKERHUB_TOKEN }} + # Turned off until user auth is fixed, see note above. + # - name: Login to DockerHub + # uses: docker/login-action@v2 + # with: + # username: ${{ secrets.DOCKERHUB_USERNAME }} + # password: ${{ secrets.DOCKERHUB_TOKEN }} - name: Login to GitHub Container Registry uses: docker/login-action@v2 with: diff --git a/docs/development/README.md b/docs/development/README.md index 98fd1efec..f94ecb513 100644 --- a/docs/development/README.md +++ b/docs/development/README.md @@ -167,5 +167,4 @@ make lint ``` You might need to install some of the linters locally, here is a list of them. -* `staticcheck`. Can be installed via `go install honnef.co/go/tools/cmd/staticcheck@latest` (assuming `$GOPATH/bin` is in your `$PATH`). * `golangci-lint`. Installation instructions provided [here](https://golangci-lint.run/usage/install/#local-installation). From 62d7a30681320183ddd859627d8d94081c7bbb67 Mon Sep 17 00:00:00 2001 From: Dmitry K Date: Mon, 26 Sep 2022 17:24:20 -0700 Subject: [PATCH 09/18] better docs --- .golangci.yml | 1 - docs/development/README.md | 41 +++++++++++++++++++++++++++++++++++--- 2 files changed, 38 insertions(+), 4 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 77ef6b3ee..d8be97a0c 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -14,7 +14,6 @@ linters: # Additional linters - gosec - - gofumpt - misspell - promlinter - unparam diff --git a/docs/development/README.md b/docs/development/README.md index f94ecb513..1c33c2f78 100644 --- a/docs/development/README.md +++ b/docs/development/README.md @@ -11,6 +11,11 @@ Please note that this repository is under very active development and breaking c - [Running LocalNet](#running-localnet) - [Code Organization](#code-organization) - [Linters](#linters) + - [Installation of golangci-lint](#installation-of-golangci-lint) + - [Running linters locally](#running-linters-locally) + - [VSCode Integration](#vscode-integration) + - [Configuration](#configuration) + - [Custom linters](#custom-linters) ## LFG - Development @@ -160,11 +165,41 @@ Pocket ### Linters -Our CI automation runs some linters and static checks to ensure code quality. You can run them locally with: +We are utilizing `golangci-lint` to run the linters. It is a wrapper around a number of linters and is configured to run many of them at once. The linters are configured to run on every commit and pull request via CI, and all code issues are populating as GitHub annotations to let developers and reviewers easily locate an issue. + +#### Installation of golangci-lint + +Please follow the instructions on the [golangci-lint](https://golangci-lint.run/usage/install/#local-installation) website. + +#### Running linters locally + +You can run `golangci-lint` locally against all packages with: ```bash make lint ``` -You might need to install some of the linters locally, here is a list of them. -* `golangci-lint`. Installation instructions provided [here](https://golangci-lint.run/usage/install/#local-installation). +If you need to specify any additional flags, you can run `golangci-lint` directly as it picks up the configuration from the `.golangci.yml` file. + +#### VSCode Integration + +`golangci-lint` has an integration with VSCode. Per [documentation](https://golangci-lint.run/usage/integrations/), recommended settings are: + +```json +"go.lintTool":"golangci-lint", +"go.lintFlags": [ + "--fast" +] +``` + +#### Configuration + +`golangci-lint` is a sophisticated tool as it includes a lot of linters, including our own custom linting rules. As a result, a configuration file might become quite large. The configuration file is located at [`.golangci.yml`](../../.golangci.yml). + +The official documentation includes a list of different linters and their configuration options. Please refer to [this page](https://golangci-lint.run/usage/linters/) for more information. + +#### Custom linters + +We can write custom linters using [`go-ruleguard`](https://go-ruleguard.github.io/). The rules are located in the [`misc/linting-rules`](../../misc/linting-rules) directory. The rules are written in the [Ruleguard DSL](https://github.com/quasilyte/go-ruleguard/blob/master/_docs/dsl.md), if you've never worked with ruleguard in the past, it makes sense to go through [introduction article](https://quasilyte.dev/blog/post/ruleguard/) and [Ruleguard by example tour](https://go-ruleguard.github.io/by-example/). + +Ruleguard is run via `gocritic` linter which is a part of `golangci-lint`, so if you wish to change configuration or debug a particular rule, you can modify the `.golangci.yml` file. From e4db9e5e6f7417cb938bc530ea0ac3e95c8765da Mon Sep 17 00:00:00 2001 From: Dmitry K Date: Mon, 26 Sep 2022 17:45:58 -0700 Subject: [PATCH 10/18] include a link to ghcr images --- docs/releases/README.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/docs/releases/README.md b/docs/releases/README.md index bdd09e175..355c95564 100644 --- a/docs/releases/README.md +++ b/docs/releases/README.md @@ -1,5 +1,13 @@ # Building & Versioning +- [Building & Versioning](#building--versioning) + - [Release Tag Versioning](#release-tag-versioning) + - [[WIP] Build Versioning](#wip-build-versioning) + - [Container Images](#container-images) + - [Tags](#tags) + - [Extended images with additional tooling](#extended-images-with-additional-tooling) + - [[TODO] Magefile build system](#todo-magefile-build-system) + ## Release Tag Versioning We follow Go's [Module Version Numbering](https://go.dev/doc/modules/version-numbers) for software releases along with typical [Software release life cycles](https://en.wikipedia.org/wiki/Software_release_life_cycle). @@ -18,6 +26,8 @@ For example, `X.Y.Z[- Date: Mon, 26 Sep 2022 18:05:22 -0700 Subject: [PATCH 11/18] turn on gocritic rules --- .github/workflows/main.yml | 7 ------- .golangci.yml | 11 +++++++++++ 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index f755dd003..cd8789501 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -58,13 +58,6 @@ jobs: uses: guyarb/golang-test-annotations@v0.5.1 with: test-results: test_results.json - # - name: Run static check - # # Only run if the test failed on target version to avoid duplicated annotations on GitHub. - # if: ${{ always() && env.TARGET_GOLANG_VERSION == matrix.go }} - # uses: reviewdog/action-staticcheck@v1 - # with: - # # TODO(@okdas): change filter after bug bash to only show annotations on files changed by PR (currently shows all changes) https://github.com/reviewdog/reviewdog#filter-mode - # filter_mode: nofilter - name: Run golangci-lint # Only run if the test failed on target version to avoid duplicated annotations on GitHub. if: ${{ always() && env.TARGET_GOLANG_VERSION == matrix.go }} diff --git a/.golangci.yml b/.golangci.yml index d8be97a0c..ad9b64df5 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -26,6 +26,17 @@ linters-settings: gocritic: enabled-checks: - ruleguard + enabled-tags: + - diagnostic + - experimental + - opinionated # This might be too much, but we can try it out + - performance + - style + disabled-checks: + - dupImport # https://github.com/go-critic/go-critic/issues/845 + - ifElseChain + # - octalLiteral + # - whyNoLint settings: ruleguard: # Enable debug to identify which 'Where' condition was rejected. From 8b338ac58768aaa443abee0ef77badc79d2635cd Mon Sep 17 00:00:00 2001 From: Dmitry Knyazev Date: Thu, 29 Sep 2022 16:36:44 -0700 Subject: [PATCH 12/18] Update misc/linting-rules/tests.go Co-authored-by: Daniel Olshansky --- misc/linting-rules/tests.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/misc/linting-rules/tests.go b/misc/linting-rules/tests.go index ff0ee5453..3b95049db 100644 --- a/misc/linting-rules/tests.go +++ b/misc/linting-rules/tests.go @@ -3,7 +3,7 @@ // +build !codeanalysis // This file includes our custom linters. -// If you want to add/modify an existing linter, please check out ruleguard documentation: https://github.com/quasilyte/go-ruleguard#documentation +// If you want to add/modify an existing linter, please check out ruleguard's documentation: https://github.com/quasilyte/go-ruleguard#documentation package gorules From ecb5580431d7bd1d769502c6298b9c9db7520932 Mon Sep 17 00:00:00 2001 From: Dmitry Knyazev Date: Thu, 29 Sep 2022 16:39:05 -0700 Subject: [PATCH 13/18] Update docs/development/README.md Co-authored-by: Daniel Olshansky --- docs/development/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/development/README.md b/docs/development/README.md index 1c33c2f78..bc959b068 100644 --- a/docs/development/README.md +++ b/docs/development/README.md @@ -165,7 +165,7 @@ Pocket ### Linters -We are utilizing `golangci-lint` to run the linters. It is a wrapper around a number of linters and is configured to run many of them at once. The linters are configured to run on every commit and pull request via CI, and all code issues are populating as GitHub annotations to let developers and reviewers easily locate an issue. +We utilize `golangci-lint` to run the linters. It is a wrapper around a number of linters and is configured to run many at once. The linters are configured to run on every commit and pull request via CI, and all code issues are populated as GitHub annotations to let developers and reviewers easily locate an issue. #### Installation of golangci-lint From de780263d68de55d9414433a1727b7efb2968d01 Mon Sep 17 00:00:00 2001 From: Dmitry Knyazev Date: Thu, 29 Sep 2022 16:39:16 -0700 Subject: [PATCH 14/18] Update docs/development/README.md Co-authored-by: Daniel Olshansky --- docs/development/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/development/README.md b/docs/development/README.md index bc959b068..feb70c5ee 100644 --- a/docs/development/README.md +++ b/docs/development/README.md @@ -194,7 +194,7 @@ If you need to specify any additional flags, you can run `golangci-lint` directl #### Configuration -`golangci-lint` is a sophisticated tool as it includes a lot of linters, including our own custom linting rules. As a result, a configuration file might become quite large. The configuration file is located at [`.golangci.yml`](../../.golangci.yml). +`golangci-lint` is a sophisticated tool including both default and custom linters. The configuration file, which can grow quite large, is located at [`.golangci.yml`](../../.golangci.yml). The official documentation includes a list of different linters and their configuration options. Please refer to [this page](https://golangci-lint.run/usage/linters/) for more information. From f4376ddf6655d7e87340667e5b2bc347a200e0a3 Mon Sep 17 00:00:00 2001 From: Dmitry Knyazev Date: Thu, 29 Sep 2022 16:39:41 -0700 Subject: [PATCH 15/18] Update docs/releases/README.md Co-authored-by: Daniel Olshansky --- docs/releases/README.md | 1 - 1 file changed, 1 deletion(-) diff --git a/docs/releases/README.md b/docs/releases/README.md index 355c95564..3ef4266d2 100644 --- a/docs/releases/README.md +++ b/docs/releases/README.md @@ -36,7 +36,6 @@ Code built from commits in Pull Requests, is tagged as `pr-`, as well as Once releases are managed, they will be tagged with the version number (e.g. `v0.0.1-alpha.pre.1`). - ### Extended images with additional tooling Extended images with additional tooling are built to aid in troubleshoot and debugging. The extended image is formatted as `-dev`. For example, `latest-dev`, or `pr-123-dev`. From a7a6f496b17d7f8bc0fb5e3defb018dc44a9d58f Mon Sep 17 00:00:00 2001 From: Dmitry Knyazev Date: Thu, 29 Sep 2022 16:39:54 -0700 Subject: [PATCH 16/18] Update docs/releases/README.md Co-authored-by: Daniel Olshansky --- docs/releases/README.md | 1 - 1 file changed, 1 deletion(-) diff --git a/docs/releases/README.md b/docs/releases/README.md index 3ef4266d2..65790ea0b 100644 --- a/docs/releases/README.md +++ b/docs/releases/README.md @@ -40,7 +40,6 @@ Once releases are managed, they will be tagged with the version number (e.g. `v0 Extended images with additional tooling are built to aid in troubleshoot and debugging. The extended image is formatted as `-dev`. For example, `latest-dev`, or `pr-123-dev`. - ## [TODO] Magefile build system Once the V1 implementation reaches the stage of testable binaries, we are looking to use [Mage](https://magefile.org/) which is being tracked in [pocket/issues/43](https://github.com/pokt-network/pocket/issues/43) that'll inject a version with the `-version` flag. From 0cbe5d75e148a4823d4d238d878a0bb3d3e45880 Mon Sep 17 00:00:00 2001 From: Dmitry K Date: Thu, 29 Sep 2022 18:05:10 -0700 Subject: [PATCH 17/18] requested changes --- .golangci.yml | 4 ++-- Makefile | 4 ++-- {misc/linting-rules => build/linters}/tests.go | 0 docs/development/README.md | 4 ++-- 4 files changed, 6 insertions(+), 6 deletions(-) rename {misc/linting-rules => build/linters}/tests.go (100%) diff --git a/.golangci.yml b/.golangci.yml index ad9b64df5..25e86bf15 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -60,10 +60,10 @@ linters-settings: # - 'dsl': gorule file does not comply with the ruleguard DSL. # Default: "" failOn: dsl - rules: "misc/linting-rules/*.go" + rules: "build/linters/*.go" run: go: "1.19" skip-dirs: - - misc/linting-rules + - build/linters build-tags: - codeanalysis diff --git a/Makefile b/Makefile index f40c8c637..d94ef06fd 100644 --- a/Makefile +++ b/Makefile @@ -326,9 +326,9 @@ test_p2p: test_p2p_addrbook: go test -run AddrBook -v -count=1 ./p2p/... -.PHONY: lint +.PHONY: go_lint ## Run all linters that are triggered by the CI pipeline -lint: +go_lint: golangci-lint run ./... .PHONY: benchmark_sortition diff --git a/misc/linting-rules/tests.go b/build/linters/tests.go similarity index 100% rename from misc/linting-rules/tests.go rename to build/linters/tests.go diff --git a/docs/development/README.md b/docs/development/README.md index feb70c5ee..891a1e943 100644 --- a/docs/development/README.md +++ b/docs/development/README.md @@ -176,7 +176,7 @@ Please follow the instructions on the [golangci-lint](https://golangci-lint.run/ You can run `golangci-lint` locally against all packages with: ```bash -make lint +make go_lint ``` If you need to specify any additional flags, you can run `golangci-lint` directly as it picks up the configuration from the `.golangci.yml` file. @@ -200,6 +200,6 @@ The official documentation includes a list of different linters and their config #### Custom linters -We can write custom linters using [`go-ruleguard`](https://go-ruleguard.github.io/). The rules are located in the [`misc/linting-rules`](../../misc/linting-rules) directory. The rules are written in the [Ruleguard DSL](https://github.com/quasilyte/go-ruleguard/blob/master/_docs/dsl.md), if you've never worked with ruleguard in the past, it makes sense to go through [introduction article](https://quasilyte.dev/blog/post/ruleguard/) and [Ruleguard by example tour](https://go-ruleguard.github.io/by-example/). +We can write custom linters using [`go-ruleguard`](https://go-ruleguard.github.io/). The rules are located in the [`build/linters`](../../build/linters) directory. The rules are written in the [Ruleguard DSL](https://github.com/quasilyte/go-ruleguard/blob/master/_docs/dsl.md), if you've never worked with ruleguard in the past, it makes sense to go through [introduction article](https://quasilyte.dev/blog/post/ruleguard/) and [Ruleguard by example tour](https://go-ruleguard.github.io/by-example/). Ruleguard is run via `gocritic` linter which is a part of `golangci-lint`, so if you wish to change configuration or debug a particular rule, you can modify the `.golangci.yml` file. From 73f116b4f04803e1d5acf683a2ad3c2a7842ac77 Mon Sep 17 00:00:00 2001 From: Dmitry K Date: Wed, 5 Oct 2022 17:04:46 -0700 Subject: [PATCH 18/18] move go_lint make target --- Makefile | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Makefile b/Makefile index d94ef06fd..0c17464f9 100644 --- a/Makefile +++ b/Makefile @@ -84,6 +84,11 @@ go_protoc-go-inject-tag: go_clean_deps: go mod tidy && go mod vendor +.PHONY: go_lint +## Run all linters that are triggered by the CI pipeline +go_lint: + golangci-lint run ./... + .PHONY: gofmt ## Format all the .go files in the project in place. gofmt: @@ -326,11 +331,6 @@ test_p2p: test_p2p_addrbook: go test -run AddrBook -v -count=1 ./p2p/... -.PHONY: go_lint -## Run all linters that are triggered by the CI pipeline -go_lint: - golangci-lint run ./... - .PHONY: benchmark_sortition ## Benchmark the Sortition library benchmark_sortition: