feat: add gofix Makefile target and CI check#2280
Conversation
📊 API Diff Results
|
There was a problem hiding this comment.
Pull request overview
Adds automation to keep go fix-driven rewrites applied consistently by introducing a make gofix target and a non-blocking CI job that checks go fix ./... is clean. Updates pkg/beholder tests to use logger.Test(t) directly (matching current go fix inlining behavior).
Changes:
- Add
gofixMakefile target (go fix ./...). - Add
check-gofixGitHub Actions job to verifymake gofixproduces no diffs (non-blocking viacontinue-on-error: true). - Apply
go fix-related inlining updates inpkg/beholdertests by replacingnewTestLogger(t)withlogger.Test(t).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| pkg/beholder/client_test.go | Switch ChipIngress test config to use logger.Test(t) directly. |
| pkg/beholder/batch_emitter_service_test.go | Replace newTestLogger(t) usage with logger.Test(t) in batch emitter tests. |
| Makefile | Add gofix target to run go fix across the current module. |
| .github/workflows/pkg.yml | Add check-gofix job to ensure make gofix leaves the working tree clean. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ./modgraph > go.md | ||
|
|
||
| .PHONY: gofix | ||
| gofix: ## Run go fix across all packages |
| go-version-file: "go.mod" | ||
| only-modules: "true" | ||
|
|
||
| - name: Ensure "make gofix" has been run |
|
|
||
| .PHONY: gofix | ||
| gofix: ## Run go fix across all packages | ||
| go fix ./... |
There was a problem hiding this comment.
We can use gomods to cover all modules
| go fix ./... | |
| gomods -go fix ./... |
- gofix now fans out across all go.mod files via gomods, so its "across all packages" description is accurate for this multi-module repo instead of only touching the root module. - check-gofix in CI only runs go fix in modules whose files actually changed, using dorny/paths-filter, to avoid unnecessary work on every push/PR. Addresses PR review comments: https://github.com/smartcontractkit/chainlink-common/pull/2280/changes#r3646569810 https://github.com/smartcontractkit/chainlink-common/pull/2280/changes#r3646569849
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (2)
.github/workflows/pkg.yml:147
- Same issue as above:
steps.filter.outputs.<name-with-hyphens>is not valid in expressions. Use bracket notation (or rename the filter keys) so the step runs when those modules change.
- name: Ensure "go fix" has been run on changed modules
if: steps.filter.outputs.root == 'true' || steps.filter.outputs.keystore == 'true' || steps.filter.outputs.observability-lib == 'true' || steps.filter.outputs.pkg-values == 'true' || steps.filter.outputs.pkg-chipingress == 'true' || steps.filter.outputs.pkg-monitoring == 'true' || steps.filter.outputs.pkg-workflows-sdk-v2-pb == 'true'
env:
.github/workflows/pkg.yml:154
- Environment variable expressions also use dot-access with hyphenated output names, which won’t resolve correctly. Use bracket notation so the shell script receives the correct "true"/"false" values.
OBSERVABILITY_LIB_CHANGED: ${{ steps.filter.outputs.observability-lib }}
PKG_VALUES_CHANGED: ${{ steps.filter.outputs.pkg-values }}
PKG_CHIPINGRESS_CHANGED: ${{ steps.filter.outputs.pkg-chipingress }}
PKG_MONITORING_CHANGED: ${{ steps.filter.outputs.pkg-monitoring }}
PKG_WORKFLOWS_SDK_V2_PB_CHANGED: ${{ steps.filter.outputs.pkg-workflows-sdk-v2-pb }}
| - name: Set up Go | ||
| if: steps.filter.outputs.root == 'true' || steps.filter.outputs.keystore == 'true' || steps.filter.outputs.observability-lib == 'true' || steps.filter.outputs.pkg-values == 'true' || steps.filter.outputs.pkg-chipingress == 'true' || steps.filter.outputs.pkg-monitoring == 'true' || steps.filter.outputs.pkg-workflows-sdk-v2-pb == 'true' | ||
| uses: ./.github/actions/setup-go |
| clientMock := mocks.NewClient(t) | ||
| clientMock.EXPECT().Close().Return(nil).Maybe() | ||
| emitter, err := beholder.NewChipIngressBatchEmitterService(clientMock, newTestConfig(), newTestLogger(t)) | ||
| emitter, err := beholder.NewChipIngressBatchEmitterService(clientMock, newTestConfig(), logger.Test(t)) |
gofixMakefile target that runsgo fix ./...check-gofixCI job that verifiesgo fix ./...produces no uncommitted changescheck-gofixwithcontinue-on-error: trueso it is non-blocking while evaluatedgo fixinlining changes topkg/beholdertests