Repository navigation
ci: run the gofmt check on every Go folder - #60
Conversation
The Gofmt check step listed three folders (cmd core transport), so an unformatted file in config, examples, protocol, security, services or tests passed CI. Run gofmt -l . instead, which is the command CONTRIBUTING.md and AGENTS.md already give. The whole tree is clean today, so the step stays green.
Reviewer's GuideConfigurationRun the CI gofmt check across the repository instead of a hand-maintained list of Go folders.
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
ⓘ Your Qodo trial ends soon. Ask your workspace admin to set up billing to keep reviews running after the trial. Manage billing |
You've used all your free Kodus-paid PR reviews 🎁Your trial is still active — this just means the PR reviews we cover during the trial are used up. Connect your own AI key to keep Kody reviewing — unlimited reviews, on any plan (Free included). Want more trial reviews to finish evaluating before adding a key? Talk to our founders. 😎 |
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 58 minutes. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. |
PR Summary by QodoCheck formatting across all Go folders in CI
AI Description
High-Level Assessment
Files changed (1)
|
Code Review by Qodo🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)
Great, no issues found!Qodo reviewed your code and found no material issues that require review |
Linked issuesChecked at commit 5f089d6 #59 CI gofmt check only covers cmd, core and transport, so unformatted code elsewhere passes · 1 of 1 met
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path=".github/workflows/ci.yml" line_range="43" />
<code_context>
+ # examples, protocol, security, services and tests.
run: |
- out=$(gofmt -l cmd core transport)
+ out=$(gofmt -l .)
if [ -n "$out" ]; then
echo "::error::files need gofmt: $out"
</code_context>
<issue_to_address>
**Skipped folders stay unchecked**
When a later change narrows the command back to a folder list and the tree has no unformatted files, a later change that restores the old folder list leaves the clean tree green, so an unformatted file in a previously skipped folder passes CI without a regression test catching it.
Add a maintained regression test that verifies the check fails for an unformatted Go file in a previously skipped folder.
</issue_to_address>Sourcery assessment
Approval pending. 1 finding to address first.
Blocking findings: .github/workflows/ci.yml:43
| # examples, protocol, security, services and tests. | ||
| run: | | ||
| out=$(gofmt -l cmd core transport) | ||
| out=$(gofmt -l .) |
There was a problem hiding this comment.
🟡 Medium · Skipped folders stay unchecked
When a later change narrows the command back to a folder list and the tree has no unformatted files, a later change that restores the old folder list leaves the clean tree green, so an unformatted file in a previously skipped folder passes CI without a regression test catching it.
Add a maintained regression test that verifies the check fails for an unformatted Go file in a previously skipped folder.
Prompt for AI agents
In `.github/workflows/ci.yml` at line 43:
**Skipped folders stay unchecked**
When a later change narrows the command back to a folder list and the tree has no unformatted files, a later change that restores the old folder list leaves the clean tree green, so an unformatted file in a previously skipped folder passes CI without a regression test catching it.
Add a maintained regression test that verifies the check fails for an unformatted Go file in a previously skipped folder.From TESTING.md L12:
- **Regression** — every fixed defect gets a test that would have caught it.
|
Premise: pass — master ci.yml:40 ran |
Summary
The
Gofmt checkstep inci.ymlnow runsgofmt -l .instead ofgofmt -l cmd core transport. The old list skipped six of the module's nine Go folders (config,examples,protocol,security,services,tests), so an unformatted file there passed CI.gofmt -l .is the commandCONTRIBUTING.mdandAGENTS.mdalready give, and the tree is clean today, so the step stays green.Type of Change
Changes
.github/workflows/ci.yml: the step's command isgofmt -l ., with a comment saying why it is not a folder list. The error message and theexit 1are unchanged.No Go code changes. The step still runs on all three OSes;
.gitattributes(*.go text eol=lf, from #42) keeps the Windows checkout LF, sogofmt -l .sees the same bytes there.Testing
The check below parses
ci.yml, takes theGofmt checkstep'srunscript exactly as written (it asserts the step exists once, in jobtest, withshell: bash), and runs it on a copy of the tree with the shell flags GitHub uses forshell: bash(bash --noprofile --norc -eo pipefail), four times: clean, with an unformatted file planted incore/(a folder the old list covered), with one planted inprotocol/(a folder it skipped), and with a file insecurity/that gofmt cannot parse. Run on macOS 26 arm64 with Go 1.26.10, the versiongo.modpins, at5f089d6.ci.yml(70748b2)check.sh <repo> ci-master.ymlprotocol/exit 0 with an unformatted file present, and thesecurity/syntax error exit 0 (never parsed). Clean tree exit 0,core/exit 1ci.ymlcheck.sh <repo> .github/workflows/ci.ymlcore/exit 1,protocol/exit 1 (::error::files need gofmt: protocol/zz_planted.go),security/syntax error exit 2 (expected ')', found '{')go build ./...go vet ./...gofmt -l .go test -race -count=1 ./...make build-allyamllint -d relaxed .github/workflows/ci.ymlgit diff --check origin/master HEADPre-Submission Checklist
Closing issue
Fixes #59
Screenshots / Logs
The step from master's
ci.ymland from this branch, on the same three trees:Additional Notes
gofmt -l .also walks anytestdata/folder. There is none today; if deliberately unformatted fixtures are added later, the step will need to skip them (for examplegofmt -l $(git ls-files '*.go' | grep -v /testdata/)).Open PR #56 stays green under the new step:
gofmt -l .on its head (5245397) prints nothing.Other CI gaps (the C SDK and its ctests are not built in CI, fuzz targets are not run) are separate and not touched here.
Summary by Sourcery
CI: