fix: ci classifier dependency files#14
Conversation
|
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 48 minutes and 44 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. 📝 WalkthroughWalkthroughThis PR introduces a new CI signal ( Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
cmd/ci_test.go (1)
66-84: Recommended: add a test for the invalid-signal rerun branch.The new rerun path in
cmd/ci.go(lines 298–300) — triggered when--dependency-files-changedreceives an unsupported value — is currently uncovered. A small case asserting exit code2,status == "rerun", and theUnsupported dependency file change signalmessage would lock in that input-validation contract.♻️ Suggested addition
func TestRunGitHubCIRerunsOnInvalidDependencyFilesChangedSignal(t *testing.T) { t.Parallel() outputDir := filepath.Join(t.TempDir(), "limier") err := runGitHubCI(t.Context(), githubCIOptions{ outputDir: outputDir, failOn: "block,rerun", dependencyFilesChanged: "bogus", getenv: emptyCIEnv, }) requireExitCode(t, err, 2) status := readCIStatus(t, filepath.Join(outputDir, "status.json")) if status.Status != "rerun" { t.Fatalf("status.Status = %q, want rerun", status.Status) } if !strings.Contains(status.Message, "Unsupported dependency file change signal") { t.Fatalf("status.Message = %q, want unsupported signal message", status.Message) } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/ci_test.go` around lines 66 - 84, Add a new unit test that exercises the invalid `--dependency-files-changed` branch by calling runGitHubCI with a githubCIOptions that sets dependencyFilesChanged to "bogus" (and failOn including "rerun"), assert the returned error exit code is 2, then read the produced status.json and assert status.Status == "rerun" and that status.Message contains "Unsupported dependency file change signal"; name the test e.g. TestRunGitHubCIRerunsOnInvalidDependencyFilesChangedSignal and follow the same pattern as TestRunGitHubCIRequiresReviewWithoutDependencyFileSignal (use t.Parallel(), a temp outputDir, emptyCIEnv, requireExitCode, and readCIStatus).docs/reference/cli.md (1)
71-73: Optional: document the accepted aliases or drop them from the parser.
resolveDependencyFilesChangeSignal(cmd/ci.go lines 359–368) also accepts1/0/yes/no(case-insensitive) as aliases fortrue/false, but the reference here only liststrue,false,unknown. Either mention the aliases or tighten the parser to only accept the three documented values, so users don't get inconsistent surface area between the CLI and the docs.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/reference/cli.md` around lines 71 - 73, The docs and parser disagree: resolveDependencyFilesChangeSignal (cmd/ci.go) accepts case-insensitive aliases "1"/"0"/"yes"/"no" in addition to "true"/"false"/"unknown" and LIMIER_CI_DEPENDENCY_FILES_CHANGED can be set to those aliases; update the CLI reference to list these accepted aliases (1, 0, yes, no) alongside true/false/unknown and mention the env var LIMIER_CI_DEPENDENCY_FILES_CHANGED, or alternatively tighten resolveDependencyFilesChangeSignal to reject any values other than "true"/"false"/"unknown" so the docs and parser match—pick one and make corresponding changes in cmd/ci.go or docs/reference/cli.md.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@cmd/ci_test.go`:
- Around line 66-84: Add a new unit test that exercises the invalid
`--dependency-files-changed` branch by calling runGitHubCI with a
githubCIOptions that sets dependencyFilesChanged to "bogus" (and failOn
including "rerun"), assert the returned error exit code is 2, then read the
produced status.json and assert status.Status == "rerun" and that status.Message
contains "Unsupported dependency file change signal"; name the test e.g.
TestRunGitHubCIRerunsOnInvalidDependencyFilesChangedSignal and follow the same
pattern as TestRunGitHubCIRequiresReviewWithoutDependencyFileSignal (use
t.Parallel(), a temp outputDir, emptyCIEnv, requireExitCode, and readCIStatus).
In `@docs/reference/cli.md`:
- Around line 71-73: The docs and parser disagree:
resolveDependencyFilesChangeSignal (cmd/ci.go) accepts case-insensitive aliases
"1"/"0"/"yes"/"no" in addition to "true"/"false"/"unknown" and
LIMIER_CI_DEPENDENCY_FILES_CHANGED can be set to those aliases; update the CLI
reference to list these accepted aliases (1, 0, yes, no) alongside
true/false/unknown and mention the env var LIMIER_CI_DEPENDENCY_FILES_CHANGED,
or alternatively tighten resolveDependencyFilesChangeSignal to reject any values
other than "true"/"false"/"unknown" so the docs and parser match—pick one and
make corresponding changes in cmd/ci.go or docs/reference/cli.md.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b08d3889-533f-434c-a79c-54caed5b5cf2
📒 Files selected for processing (4)
cmd/ci.gocmd/ci_test.godocs/guide/ci-and-deploy.mddocs/reference/cli.md
1 participant
Summary by CodeRabbit
Release Notes
New Features
--dependency-files-changedflag andLIMIER_CI_DEPENDENCY_FILES_CHANGEDenvironment variable to classify dependency file changes in CI workflows (accepted values:true,false,unknown).Behavior Updates
Documentation