Skip to content

chore: golangci-lint config + test race fixes + -race CI#13

Open
parkan wants to merge 4 commits into
fil-forge:mainfrom
parkan:chore/lint-and-race-fixes
Open

chore: golangci-lint config + test race fixes + -race CI#13
parkan wants to merge 4 commits into
fil-forge:mainfrom
parkan:chore/lint-and-race-fixes

Conversation

@parkan

@parkan parkan commented May 28, 2026

Copy link
Copy Markdown

three commits, take what you think makes sense (I think all 3 are good!)

  • chore: add golangci-lint config -- tuned .golangci.yml (v2), gosec noise excluded, CI-runnable on public runners.
  • fix: race conditions in test code -- worker/preparation tests share state across goroutines; counters/flags -> atomics, caps slices mutex-guarded, tests only
  • ci: enable -race in test workflow -- flips go test to -race, can reject but I would say turn this on


- name: Test
run: go test -v -cover ./...
run: go test -race -v -cover ./...

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We typically run tests without and then with in a separate step. Could we do that instead?

"Test with race detector"

@frrist frrist left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this! The race check is a real improvement. Two things before merging:

1. Adopt the shared fil-forge CI workflows instead of bespoke ones.
At least (probably more) sprue, piri, and libforge all drive testing and checks through ipdxco/unified-github-workflows@v1.0, via three files that are identical across those repos: go-test.yml, go-check.yml, and go-test-config.json. Borrowing them here gets us -race + coverage/codecov + concurrency-cancel from go-test, and gofmt/vet/staticcheck/golangci-lint from go-check, replacing both the manual -race edit and the bespoke test.yml/check.yml. The aim is for every fil-forge repo to run the same flow.

2. Hold the .golangci.yml for now.
I'm for adding a linter, but as it stands the config does nothing: go-check only runs golangci-lint when a config is present, and guppy isn't on go-check yet so nothing currently invokes it (or am I missing something? I don't think the current check runs this). The catch is that the moment we adopt the shared workflows from point 1, this config gets picked up automatically, quietly switching on every rule in here at once (or maybe loudly if a bunch fail). I'd rather not have the whole set go live as a side effect of a CI migration. So before the lint config lands, I'd like to:

  • Trim the rule set. There's a lot here, and some rules want team buy-in, e.g. I've personally found gocritic and gocyclo more annoying than helpful, but that's a team call.
  • Roll it out across all fil-forge repos together, ideally from one shared config (though the per-path exclusions may make a single shared file awkward). Once we settle that, dropping the agreed config into any repo already on go-check wires it up automatically w/ consistency across all projects.

@parkan

parkan commented Jun 17, 2026

Copy link
Copy Markdown
Author

@frrist sorry for the delay in responding here, had some personal things, will reply properly asap

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants