Conversation
- Add missing imports - Optimize string concatenation using strings.Builder - Use Go 1.22 range-over-int syntax
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 14 minutes and 43 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis pull request systematically modernises the codebase to use Go's Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
- Update CI matrix go version from '1' (resolving to 1.25.8) to 'stable' so it meets the go.mod requirement of go >= 1.26.0 - Add table-driven tests for toSnakeCase, IDZeroValue, parseUpdaters, and parseInserters to cover functions changed in this PR; coverage on those functions goes from 0% to 100% Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
.github/workflows/test.yml (1)
18-18: Consider testing against the minimum supported Go version as well.The workflow now only tests against the latest stable Go release. To ensure the codebase remains compatible with the minimum Go version specified in go.mod (1.26.0), consider adding it to the test matrix:
go: ['1.26.0', 'stable']This would verify that the code doesn't accidentally rely on features introduced in Go versions newer than 1.26.0.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/test.yml at line 18, Update the CI test matrix in the workflow to include the minimum supported Go version from go.mod: modify the existing go matrix entry (currently "go: ['stable']") to include "1.26.0" so it reads like go: ['1.26.0', 'stable']; this ensures the workflow tests both the declared minimum Go version and the stable release.shiftgen/shiftgen.go (1)
150-152: Skip empty tokens while parsing comma-separated flag values.Both parsers currently append trimmed entries even when they are empty (e.g. trailing commas or
", ,"). Filtering blanks here will produce clearer downstream behaviour.Proposed patch
func parseInserters() ([]string, error) { @@ } else if strings.TrimSpace(*inserters) != "" { for i := range strings.SplitSeq(*inserters, ",") { - ii = append(ii, strings.TrimSpace(i)) + v := strings.TrimSpace(i) + if v == "" { + continue + } + ii = append(ii, v) } } return ii, nil } @@ func parseUpdaters() []string { var uu []string if strings.TrimSpace(*updaters) != "" { for u := range strings.SplitSeq(*updaters, ",") { - uu = append(uu, strings.TrimSpace(u)) + v := strings.TrimSpace(u) + if v == "" { + continue + } + uu = append(uu, v) } } return uu }Also applies to: 160-162
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shiftgen/shiftgen.go` around lines 150 - 152, The token appending loops using strings.SplitSeq(*inserters, ",") currently append trimmed tokens even when empty; change the loops (the one building ii from inserters and the similar loop around lines 160-162) to trim each token with strings.TrimSpace, then skip adding it if the result is an empty string (continue), so only non-blank tokens are appended to ii (and the other target slice).shiftgen/shiftgen_test.go (1)
56-67: Consider adding comma-edge cases to pin parser behaviour.An extra case for inputs like
"A,,B"or"A,"would make empty-token handling explicit and prevent future regressions.Also applies to: 88-93
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shiftgen/shiftgen_test.go` around lines 56 - 67, Add explicit comma-edge test cases to TestParseUpdaters to assert how empty tokens are handled: include inputs like "A,,B" expecting either ["A","B"] or an error/nil as per ParseUpdaters' contract, and inputs like "A," and ",A" to verify trailing/leading comma behavior; locate the test table in TestParseUpdaters and append rows for these scenarios (also mirror the same additions in the other table around lines 88-93) so the parser's handling of empty tokens is pinned down.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/workflows/test.yml:
- Line 18: Update the CI test matrix in the workflow to include the minimum
supported Go version from go.mod: modify the existing go matrix entry (currently
"go: ['stable']") to include "1.26.0" so it reads like go: ['1.26.0', 'stable'];
this ensures the workflow tests both the declared minimum Go version and the
stable release.
In `@shiftgen/shiftgen_test.go`:
- Around line 56-67: Add explicit comma-edge test cases to TestParseUpdaters to
assert how empty tokens are handled: include inputs like "A,,B" expecting either
["A","B"] or an error/nil as per ParseUpdaters' contract, and inputs like "A,"
and ",A" to verify trailing/leading comma behavior; locate the test table in
TestParseUpdaters and append rows for these scenarios (also mirror the same
additions in the other table around lines 88-93) so the parser's handling of
empty tokens is pinned down.
In `@shiftgen/shiftgen.go`:
- Around line 150-152: The token appending loops using
strings.SplitSeq(*inserters, ",") currently append trimmed tokens even when
empty; change the loops (the one building ii from inserters and the similar loop
around lines 160-162) to trim each token with strings.TrimSpace, then skip
adding it if the result is an empty string (continue), so only non-blank tokens
are appended to ii (and the other target slice).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 39bd9b7c-dcac-4f5a-9ddd-14d883574f4d
📒 Files selected for processing (14)
.github/workflows/test.ymlarc.gogen_1_test.gogen_2_test.gogen_3_test.gogen_4_test.gogen_string_test.gohelper_test.goshift.goshift_internal_test.goshiftgen/shiftgen.goshiftgen/shiftgen_test.gotest_shift.gotest_shift_test.go
|
1 participant
Apply
go fixto modernize the codebase:strings.Builder(performance improvement over concatenation loops)for i := range Ninstead offor i := 0; i < N; i++)Summary by CodeRabbit
Release Notes
Chores
Tests