Skip to content

Commit

Permalink
Consolidate lint steps in GitHub Actions (#231)
Browse files Browse the repository at this point in the history
Like with running Go tests, the lints for each separate Go module must
be running separately. This is currently being accomplished in CI by
giving each Go module its own lint job using golangci-lint's GitHub
Action.

Unfortunately, this is really not working out well. Although the lints
themselves are all very fast to run (just a couple seconds), the post
run step where it saves it cache is quite slow, about two minutes, and
that's happening for each of the lint steps, making the entire lint job
take almost _ten minutes_ to run.

I think what might've been happening is that each lint job was
overwriting the last job's cache, which is why each post run step seemed
to be doing so much work. I didn't validate this hypothesis, but it
seems like a strong possibility.

Here, try to hack around the problem by having the main lint job fetch
golangci-lint, but then only run `--help`, and then do the real linting
as a separate step (one that doesn't use the GitHub Action) that calls
into our `make lint` target to run lints for each Go module.

A downside is that it may not annotate lint problems on the GitHub PR,
which is something that theoretically happened before, although it never
seemed to work for me. This might not be that bad though because it
could as a side effect improve the log output, which is terrible for the
GitHub Action step because it doesn't include files or line numbers.

Also, I notice that the lints/tests in `Makefile` had been commented out
for `./cmd/river` for some reason (maybe something I did during the
driver refactor and forgot to fix), so I uncommented them.

[1] golangci/golangci-lint-action#271
  • Loading branch information
brandur committed Feb 29, 2024
1 parent 4e5a6f8 commit 0ace41d
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 32 deletions.
40 changes: 10 additions & 30 deletions .github/workflows/ci.yaml
Expand Up @@ -163,38 +163,18 @@ jobs:
- name: Lint
uses: golangci/golangci-lint-action@v4
with:
# golangci-lint needs to be run separately for every Go module, and
# its GitHub Action doesn't provide any way to do that. Have it fetch
# the golangci-lint binary, trick it into not running by sending only
# `--help`, then run the full set of lints below. DO NOT run separate
# modules as separate golangci-lint-action steps. Its post run caching
# can be extremely slow, and that's amplified in a very painful way if
# it needs to be run multiple times.
args: --help
version: ${{ env.GOLANGCI_LINT_VERSION }}
working-directory: .

- name: Lint cmd/river
uses: golangci/golangci-lint-action@v4
with:
version: ${{ env.GOLANGCI_LINT_VERSION }}
working-directory: ./cmd/river

- name: Lint riverdriver
uses: golangci/golangci-lint-action@v4
with:
version: ${{ env.GOLANGCI_LINT_VERSION }}
working-directory: ./riverdriver

- name: Lint riverdriver/riverdatabasesql
uses: golangci/golangci-lint-action@v4
with:
version: ${{ env.GOLANGCI_LINT_VERSION }}
working-directory: ./riverdriver/riverdatabasesql

- name: Lint riverdriver/riverpgxv5
uses: golangci/golangci-lint-action@v4
with:
version: ${{ env.GOLANGCI_LINT_VERSION }}
working-directory: ./riverdriver/riverpgxv5

- name: Lint rivertype
uses: golangci/golangci-lint-action@v3
with:
version: ${{ env.GOLANGCI_LINT_VERSION }}
working-directory: ./rivertype
- name: Run lint
run: make lint

producer_sample:
runs-on: ubuntu-latest
Expand Down
4 changes: 2 additions & 2 deletions Makefile
Expand Up @@ -10,7 +10,7 @@ generate/sqlc:
.PHONY: lint
lint:
cd . && golangci-lint run --fix
# cd cmd/river && golangci-lint run --fix
cd cmd/river && golangci-lint run --fix
cd riverdriver && golangci-lint run --fix
cd riverdriver/riverdatabasesql && golangci-lint run --fix
cd riverdriver/riverpgxv5 && golangci-lint run --fix
Expand All @@ -19,7 +19,7 @@ lint:
.PHONY: test
test:
cd . && go test ./... -p 1
# cd cmd/river && go test ./...
cd cmd/river && go test ./...
cd riverdriver && go test ./...
cd riverdriver/riverdatabasesql && go test ./...
cd riverdriver/riverpgxv5 && go test ./...
Expand Down

0 comments on commit 0ace41d

Please sign in to comment.