ci(stale): drop phantom priority/critical exempt label - #1925
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Enterprise Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughUpdated the stale-management workflow to remove Estimated code review effort: 1 (Trivial) | ~2 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
4182451 to
7b2b342
Compare
7b2b342 to
5c5d013
Compare
5c5d013 to
0ceca53
Compare
The stale workflow exempts priority/critical from auto-close, but no priority/* label exists in this repository, so the entry can never match. actions/stale matches label names only and cannot read Projects v2 fields, where priority actually lives (P0/P1/P2 on the AICR board), so the label was never going to deliver automatic exemption. lifecycle/frozen already provides the freeze mechanism, sits in the same exempt-issue-labels list, and covers pull requests via exempt-pr-labels. Also corrects the same comment block's description of do-not-merge. exempt-pr-labels excludes a PR from being marked stale at all, so it never receives the label or the warning comment - it is not merely spared the close. That understatement is the framing NVIDIA#1921 replaced in CONTRIBUTING.md for the same mechanism. No behavior change: the removed entry never fired, and the other edit is a comment. Related: NVIDIA#1920, NVIDIA#1921 Signed-off-by: Yuan Chen <yuanchen97@gmail.com>
0ceca53 to
1fc799e
Compare
Summary
Removes
priority/criticalfromexempt-issue-labelsin.github/workflows/stale.yaml, and the matching line from the comment block above it. No such label exists in this repository, so the entry can never match.Also corrects that same comment block's one-line description of
do-not-merge, which understated the exemption in the way #1921 just fixed inCONTRIBUTING.md.Motivation / Context
This is a proposal, not a settled decision.
priority/criticalis inert either way — an unmatched exempt label simply never fires — so there are two defensible resolutions, and this PR implements one of them:lifecycle/frozenis the intended freeze mechanism.The decisive point is that
actions/stalecan only match label names. It has no access to Projects v2 fields. Priority in this repo lives on the AICR project board as aPriorityfield (P0/P1/P2), not as repo labels, which is why nopriority/*label was ever created. So creatingpriority/criticalwould not give board-P0 issues automatic exemption — someone would still hand-apply the label to each one. That collapses it into a synonym forlifecycle/frozen, which already exists, already sits in the sameexempt-issue-labelslist, and already covers pull requests too viaexempt-pr-labels.The trade-off is therefore not "protection versus no protection" — it is "one way to freeze an issue, or two."
Why fix it at all, given it is inert. The entry is documentation-shaped. The comment at
:26reads as a statement of policy, and it is where the incorrectCONTRIBUTING.mdclaim originated. #1921 removed that claim from the docs; this removes the source. Left alone, the next person auditing the workflow re-documents it and the same error returns.Evidence:
gh api repos/NVIDIA/aicr/labels --paginatereturns nopriority/*label of any kind. The other two exempt labels,lifecycle/frozenandgood first issue, both exist and work.Fixes: N/A
Related: #1920, #1921
This addresses one bullet of #1920 and deliberately does not close it — that issue also covers the
aicr-triageClose-bucket label rule and the "Stalled" naming collision, both of which are independent follow-ups.Type of Change
Component(s) Affected
cmd/aicr,pkg/cli)cmd/aicrd,pkg/server)pkg/recipe)pkg/bundler,pkg/component/*)pkg/collector,pkg/snapshotter)pkg/validator)pkg/errors,pkg/k8s)docs/,examples/).github/workflows/stale.yamlImplementation Notes
No behavior change. The removed label never matched, so no issue's staleness handling differs before or after.
lifecycle/frozenandgood first issuecontinue to exempt issues;lifecycle/frozenanddo-not-mergecontinue to exempt pull requests.The
do-not-mergecomment was understated.exempt-pr-labels(:80) isactions/stale's exemption list, so an exempt PR is never marked stale — it receives neither thelifecycle/stalelabel nor the warning comment. It is not merely spared the close. "PR won't be closed" is precisely the framing #1921 replaced inCONTRIBUTING.md("To prevent auto-close" → "To stay out of the stale process entirely") for this same mechanism, so leaving it would have put the workflow comment and the documentation in disagreement about the same behavior — the drift this PR exists to prevent.Scope is deliberately narrow: three lines, all inside the same comment block or the config entry it describes. The documentation half was fixed separately in #1921, and the
aicr-triageskill changes from #1920 belong in their own PR against the skill file.One adjacent gap left alone. The comment block still omits
good first issue, so it remains an incomplete summary of:66after this change. That is incompleteness rather than an incorrect statement, which is why it is treated differently from thedo-not-mergewording above. Happy to add it if a reviewer prefers the block be exhaustive.Reversible. If a maintainer later decides critical issues warrant a dedicated exemption, restoring it is one API call to create the label plus these two lines back. Deleting a dead string forecloses nothing.
Testing
make qualifywas not run: this changes one CI workflow file and no Go, YAML-under-test, or documentation source, so the repository gate cannot regress from it. The relevant check isactionlint, which passes locally and also runs in CI on.github/**changes.Risk Assessment
Rollout notes: No migration. The removed entry had no effect, so the stale bot behaves identically before and after.
Checklist
make testwith-race) — N/A, no Go changes;actionlintpassesmake lint) —actionlintclean on the changed filegit commit -S) — GPG signing info