Repository navigation
feat(standards): require -reusable.yml naming suffix for pure reusables - #627
Conversation
Layer a naming convention on top of the disable standard: a pure reusable (workflow_call-only) MUST be named `*-reusable.yml` so its "library, safe to disable, never runnable on its own" nature is legible from the filename without opening the file. The suffix is a readability aid, NOT the detection mechanism — the compliance check still classifies by triggers, never by name (a name can lie, which is exactly how the pr-review.yml engine hid as a pure reusable). - ci-standards.md: add the naming requirement + rationale to the "Pure reusable workflows must be disabled" section; document the grandfathered pr-review.yml exception (tracked in .github-private#1127). - compliance-audit.sh: check_reusable_workflows_disabled() now also emits a separate `reusable-naming-<wf>` warning for any pure reusable lacking the suffix, independent of the disabled-state finding. pr-review.yml is grandfathered until its rename lands (#1127). 15/16 pure reusables already comply; simulated across the live fleet, the only non-compliant name is the grandfathered pr-review.yml (suppressed). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR adds a suffix-based naming lint for pure ChangesReusable Workflow Naming Rule
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a naming standard for pure reusable workflows, requiring them to use the -reusable.yml suffix. This standard is documented in standards/ci-standards.md and enforced via a new lint check in scripts/compliance-audit.sh, with a grandfathered exception for the legacy pr-review.yml workflow. The review feedback suggests improving the Bash script by consistently using double brackets [[ ... ]] for conditional checks to avoid potential word splitting or globbing issues.
There was a problem hiding this comment.
Pull request overview
Adds an org-wide naming convention for “pure reusable” GitHub Actions workflows (those with on: workflow_call only), requiring a *-reusable.yml suffix to make “library-only” workflows recognizable at a glance, and extends the compliance audit to emit a separate naming warning (with a documented/suppressed legacy exception for .github-private/pr-review.yml).
Changes:
- Document
*-reusable.ymlnaming requirement for pure reusables (plus rationale and a grandfathered exception). - Extend
check_reusable_workflows_disabled()to emitreusable-naming-*warnings when a pure reusable lacks the suffix. - Suppress the naming warning for the legacy
.github-private/pr-review.ymlengine pending its tracked rename.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| standards/ci-standards.md | Documents the -reusable.yml naming requirement for pure reusables, including rationale and the pr-review.yml grandfathered exception. |
| scripts/compliance-audit.sh | Adds a naming-lint warning for pure reusable workflows that don’t use the -reusable.yml suffix, with a temporary suppression for .github-private/pr-review.yml. |
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@standards/ci-standards.md`:
- Around line 169-183: The Compliance paragraph is stale and still describes
only active-state workflow findings, so update it to match the current audit
behavior. In the standards/ci-standards.md Compliance section, revise the
description to mention both the new reusable naming check for workflows that
should use the -reusable.yml suffix and the independent reusable-naming-<wf>
warning emitted by the script, so the docs stay aligned with the script’s
current checks and the reusable naming rule referenced elsewhere in the
document.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: d0b9be32-936e-4ce2-abde-32fce59b07a2
📒 Files selected for processing (2)
scripts/compliance-audit.shstandards/ci-standards.md
|
@coderabbitai resolve |
Dev-Lead — review-changes (applied)Changes committed and pushed. |
✅ Action performedComments resolved and changes approved. |
Dev-Lead — review-changes (applied)Changes committed and pushed. |
Dev-Lead — fix-bot-comment (no-changes)Agent reasoning |
donpetry-bot
left a comment
donpetry-bot
left a comment
There was a problem hiding this comment.
Automated review — APPROVED ✓
Risk: LOW
Reviewed commit: 07d8efad364438cc25e7f00dbf06c857c73caad3
Review mode: triage-approved (single reviewer)
Summary
Follow-up to #626 adding a naming convention for pure reusable workflows: warning-level lint in compliance-audit.sh requiring the -reusable.yml suffix, matching standards doc update, and 5 new bats tests. Additive, well-scoped, all CI green, all review threads resolved. Confirms the triage low-risk assessment.
Linked issue analysis
No closing issue linked. The PR is an explicitly documented follow-up to merged #626 (the naming rule was approved in that PR's discussion but landed after auto-merge). The one non-compliant workflow (pr-review.yml) is grandfathered with its rename tracked in petry-projects/.github-private#1127, referenced both in the doc and the script comment.
Findings
- No blocking findings.
- scripts/compliance-audit.sh: naming lint is additive (separate warning finding), correctly placed after the hybrid-trigger exemption, and classification remains trigger-based per the stated design. Grandfather match is repo-scoped (.github-private/pr-review.yml) and covered by a dedicated bats test.
- standards/ci-standards.md: naming requirement, rationale, and grandfathered exception documented; compliance-check section updated to match (the earlier CodeRabbit stale-doc finding was addressed and the thread resolved).
- test: 5 new bats tests mirror the check, including suffix-compliant, missing suffix, grandfather scoping, and .yaml-vs-.yml edge case.
- Secret scan: run_secret_scanning MCP tool unavailable in this run; gitleaks CI check passed and the diff contains no secret-like content.
- Prior CodeRabbit CHANGES_REQUESTED review was dismissed and superseded by a CodeRabbit APPROVED review; all 3 review threads resolved.
CI status
All required checks green: ShellCheck, Lint and bats, CodeQL, Agent Security Scan, Secret scan (gitleaks), SonarCloud (quality gate passed), agent-shield, npm audit. Ecosystem-specific audits skipped (not applicable). An early cancelled dev-lead dispatch run was superseded by a later SUCCESS. Branch is BEHIND main but MERGEABLE.
Reviewed automatically by the PR-review agent (single-reviewer mode: fable 5). Reply if you need a human review.
|
Dev-Lead — fix-bot-comment (no-changes)Agent reasoning |
…es (#627) * feat(standards): require -reusable.yml naming suffix for pure reusables Layer a naming convention on top of the disable standard: a pure reusable (workflow_call-only) MUST be named `*-reusable.yml` so its "library, safe to disable, never runnable on its own" nature is legible from the filename without opening the file. The suffix is a readability aid, NOT the detection mechanism — the compliance check still classifies by triggers, never by name (a name can lie, which is exactly how the pr-review.yml engine hid as a pure reusable). - ci-standards.md: add the naming requirement + rationale to the "Pure reusable workflows must be disabled" section; document the grandfathered pr-review.yml exception (tracked in .github-private#1127). - compliance-audit.sh: check_reusable_workflows_disabled() now also emits a separate `reusable-naming-<wf>` warning for any pure reusable lacking the suffix, independent of the disabled-state finding. pr-review.yml is grandfathered until its rename lands (#1127). 15/16 pure reusables already comply; simulated across the live fleet, the only non-compliant name is the grandfathered pr-review.yml (suppressed). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore: dev-lead update (review-changes) [skip ci-relay] * chore: dev-lead update (review-changes) [skip ci-relay] --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: donpetry-bot <281750570+donpetry-bot@users.noreply.github.com> Co-authored-by: Don Petry Bot <donpetry+bot@gmail.com>
What
Follow-up to #626 (merged). Layers a naming convention on top of the just-landed "pure reusable workflows must be disabled" standard: a pure reusable (
on: workflow_callonly) MUST be named*-reusable.yml.This was reviewed and approved as part of #626's discussion but landed after that PR auto-merged, so it comes in as its own small PR.
Why
The
-reusable.ymlsuffix makes the "this is a library, safe to disable, never runnable on its own" property legible from the filename — no need to open the file and parseon:. The suffix is a readability aid, not the detection mechanism: the compliance check still classifies by triggers, never by name (a name can lie — exactly how thepr-review.ymlengine hid as a pure reusable).Changes
standards/ci-standards.md— add the naming requirement + rationale to the "Pure reusable workflows must be disabled" section; document the grandfatheredpr-review.ymlexception.scripts/compliance-audit.sh—check_reusable_workflows_disabled()now also emits a separatereusable-naming-<wf>warning for any pure reusable lacking the suffix, independent of the disabled-state finding.pr-review.ymlis grandfathered until its rename lands.Grandfather / follow-up
pr-review.ymlis the one pure reusable (15/16 already comply) without the suffix; renaming it is entangled with its caller path + 10pr-review/*release-channel tags. Tracked separately in petry-projects/.github-private#1127; suppressed in the check until then.Validation
bash -n+shellcheck -S warningclean.reusable-workflows-disabled.bats, added in feat(standards): require pure reusable workflows to be disabled + compliance check #626) still passes — naming block is additive.pr-review.yml.🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
workflow_call-only workflows must end with-reusable.yml).Documentation
-reusable.ymlnaming requirement and the required manual disablement in the GitHub Actions UI.Tests