chore: add CI linting, sync deps, fix lint issues#13
Conversation
- Update options dependency v0.2.3 to v0.2.4 - Fix unchecked lgr.Log() errors in gokit logger - Add golangci-lint config and CI lint job - Standardize CI with separate build/test/lint jobs
📝 WalkthroughWalkthroughThe pull request introduces CI/build infrastructure improvements including GitHub workflow refactoring with explicit permissions and job separation, adds golangci-lint configuration, extends the Makefile with linting and benchmarking targets, updates a Go module dependency version, and applies explicit error handling in the gokit logger. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
This PR introduces CI linting via golangci-lint, refreshes dependencies, and addresses lint findings in the go-kit logger implementation to standardize the project’s build/test/lint automation.
Changes:
- Bump
github.com/go-coldbrew/optionsfromv0.2.3tov0.2.4(and syncgo.sum). - Add
.golangci.yml, amake linttarget, and a GitHub Actions lint job. - Update go-kit logger usage to satisfy error-checking linters.
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
loggers/gokit/gokit.go |
Adjusts go-kit logger calls to handle/acknowledge Log() return errors. |
go.mod |
Updates the options dependency version. |
go.sum |
Syncs module checksums after dependency changes. |
Makefile |
Adds lint and bench targets; expands .PHONY. |
.golangci.yml |
Adds golangci-lint configuration. |
.github/workflows/go.yml |
Splits CI into separate build/test/lint jobs and wires up golangci-lint. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Add permissions: contents: read to CI workflows - Fix go test flag order (-race before packages) - Fix bench target to skip regular tests (-run=^$)
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 6 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Pin actions/checkout, actions/setup-go, golangci-lint-action to commit SHAs - Replace hardcoded go-version with go-version-file: go.mod
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 6 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Make lint target reproducible (go install golangci-lint) - Add missing tests and strengthen assertions - Fix error string grammar
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
loggers/gokit/gokit.go (1)
36-40: Handlelgr.Logerrors instead of discarding them.On Line 37 and Line 39,
_ = lgr.Log(...)only suppresses lint and still hides sink failures. Prefer a best-effort fallback path so logging outages are visible.Proposed diff
if len(args) == 1 { - _ = lgr.Log("msg", args[0]) + if err := lgr.Log("msg", args[0]); err != nil { + _, _ = fmt.Fprintf(os.Stderr, "gokit logger write failed: %v\n", err) + } } else { - _ = lgr.Log(args...) + if err := lgr.Log(args...); err != nil { + _, _ = fmt.Fprintf(os.Stderr, "gokit logger write failed: %v\n", err) + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@loggers/gokit/gokit.go` around lines 36 - 40, The calls to lgr.Log in the logging helper currently swallow returned errors (`_ = lgr.Log(...)`); change this to capture the error value from lgr.Log (the calls around the conditional using lgr.Log and variable lgr) and implement a best-effort fallback when err != nil such as writing an error message (including the original log args and the returned err) to a safe sink (e.g., os.Stderr or the std library logger) so sink failures are visible; update both branches (the single-arg branch that logs "msg", args[0] and the variadic branch that forwards args...) to follow this pattern.Makefile (1)
13-15: Consider pinning golangci-lint version for reproducibility.Using
@latestworks but may cause inconsistent behavior across different developer machines or CI runs over time. Consider pinning to a specific version (e.g.,@v2.1.6) that matches or is compatible with theversion: v2.8specified in the GitHub Actions workflow.♻️ Suggested change
lint: - go install github.com/golangci/golangci-lint/cmd/golangci-lint@latest + go install github.com/golangci/golangci-lint/cmd/golangci-lint@v2.1.6 golangci-lint run🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Makefile` around lines 13 - 15, The Makefile's lint target installs golangci-lint with `@latest` making builds unreproducible; change the install command in the lint target to pin a specific version (e.g., replace "go install github.com/golangci/golangci-lint/cmd/golangci-lint@latest" in the lint target with a fixed tag such as "@v2.8.0" or the version that matches CI) so all developers and CI use the same golangci-lint binary.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/go.yml:
- Around line 53-57: Update the golangci-lint GitHub Action to a newer release
by changing the action input for the "Lint" step (the step with name "Lint" and
uses: golangci/golangci-lint-action@...) so the version under with: version is
at least v2.11 (e.g., v2.11 or v2.11.3) instead of v2.8; keep the args and
timeout unchanged and ensure the updated version string follows the existing
format used in the workflow.
---
Nitpick comments:
In `@loggers/gokit/gokit.go`:
- Around line 36-40: The calls to lgr.Log in the logging helper currently
swallow returned errors (`_ = lgr.Log(...)`); change this to capture the error
value from lgr.Log (the calls around the conditional using lgr.Log and variable
lgr) and implement a best-effort fallback when err != nil such as writing an
error message (including the original log args and the returned err) to a safe
sink (e.g., os.Stderr or the std library logger) so sink failures are visible;
update both branches (the single-arg branch that logs "msg", args[0] and the
variadic branch that forwards args...) to follow this pattern.
In `@Makefile`:
- Around line 13-15: The Makefile's lint target installs golangci-lint with
`@latest` making builds unreproducible; change the install command in the lint
target to pin a specific version (e.g., replace "go install
github.com/golangci/golangci-lint/cmd/golangci-lint@latest" in the lint target
with a fixed tag such as "@v2.8.0" or the version that matches CI) so all
developers and CI use the same golangci-lint binary.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2da8b129-d6cf-4769-8c96-3ccd36033565
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (5)
.github/workflows/go.yml.golangci.ymlMakefilego.modloggers/gokit/gokit.go
Summary
lgr.Log()errors in gokit loggerTest plan
make buildpassesmake testpasses (with -race)make lintpasses with 0 issuesSummary by CodeRabbit
Release Notes
Chores
New Features
lintandbenchtargets to build commands for local development.