feat: require customer impact section in PRs#1154
Open
sarinali wants to merge 1 commit intotrycua:mainfrom
Open
feat: require customer impact section in PRs#1154sarinali wants to merge 1 commit intotrycua:mainfrom
sarinali wants to merge 1 commit intotrycua:mainfrom
Conversation
Adds a PR template with Customer Impact and Visibility sections, plus a GitHub Action that blocks merging if the impact description is missing or no visibility checkbox (External/Internal) is checked. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
|
@sarinali is attempting to deploy a commit to the Cua Team on Vercel. A member of the Team first needs to authorize it. |
Contributor
📝 WalkthroughWalkthroughTwo new files establish a standardized PR process: a template defining required sections including Customer Impact with visibility checkboxes, and a GitHub Actions workflow that validates the template's Customer Impact section is completed with at least one visibility option selected. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 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 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/ci-check-customer-impact.yml:
- Around line 18-22: The current extraction leaves the "### Visibility" scaffold
and its checklist in impactContent so stripped still appears non-empty; update
the cleaning step (the code that assigns stripped, which currently does
impactContent.replace(/<!--[\s\S]*?-->/g, '').trim()) to also remove the
Visibility section and any checklist lines: after stripping HTML comments, run a
regex to remove a "### Visibility" heading plus its following checkbox lines
(e.g. /###\s*Visibility[\s\S]*?(?=\n###|\n##|\n*$)/i) and/or remove standalone
checklist lines like /^\s*-\s*\[[ xX]\]\s*.*$/gm, then trim—so impactContent ->
stripped no longer counts the visibility scaffold as customer-impact text.
- Around line 24-27: The current visibility check (externalChecked,
internalChecked, hasVisibility) allows both boxes to be checked; change the
validation to require exactly one selection by replacing hasVisibility with a
check using XOR semantics (e.g., const exactlyOneVisibility = externalChecked
!== internalChecked) and treat any other case (neither or both true) as a
validation failure that fails the CI job and emits a clear error message
instructing the user to "Check one: External OR Internal" (reference
externalChecked, internalChecked, and the new exactlyOneVisibility variable).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 429f6fa0-2176-4056-8379-caf8538d50c9
📒 Files selected for processing (2)
.github/PULL_REQUEST_TEMPLATE.md.github/workflows/ci-check-customer-impact.yml
Comment on lines
+18
to
+22
| const impactMatch = body.match(/## Customer Impact\s*\n([\s\S]*?)(?=\n## |\n*$)/); | ||
| const impactContent = impactMatch ? impactMatch[1].trim() : ''; | ||
|
|
||
| // Strip HTML comments | ||
| const stripped = impactContent.replace(/<!--[\s\S]*?-->/g, '').trim(); |
Contributor
There was a problem hiding this comment.
Don't count the visibility scaffold as customer-impact text.
Line 22 only removes HTML comments. The ### Visibility heading and checkbox labels are still left in stripped, so an untouched template — or a PR with only - [x] Internal and no explanation — will bypass the empty-impact check at Line 31.
Suggested fix
- const stripped = impactContent.replace(/<!--[\s\S]*?-->/g, '').trim();
+ const stripped = impactContent
+ .replace(/<!--[\s\S]*?-->/g, '')
+ .replace(/^### Visibility\s*$/gim, '')
+ .replace(/^- \[[ xX]\] (External|Internal)\b.*$/gim, '')
+ .trim();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/ci-check-customer-impact.yml around lines 18 - 22, The
current extraction leaves the "### Visibility" scaffold and its checklist in
impactContent so stripped still appears non-empty; update the cleaning step (the
code that assigns stripped, which currently does
impactContent.replace(/<!--[\s\S]*?-->/g, '').trim()) to also remove the
Visibility section and any checklist lines: after stripping HTML comments, run a
regex to remove a "### Visibility" heading plus its following checkbox lines
(e.g. /###\s*Visibility[\s\S]*?(?=\n###|\n##|\n*$)/i) and/or remove standalone
checklist lines like /^\s*-\s*\[[ xX]\]\s*.*$/gm, then trim—so impactContent ->
stripped no longer counts the visibility scaffold as customer-impact text.
Comment on lines
+24
to
+27
| // Check that at least one visibility checkbox is checked | ||
| const externalChecked = /- \[x\] External/i.test(impactContent); | ||
| const internalChecked = /- \[x\] Internal/i.test(impactContent); | ||
| const hasVisibility = externalChecked || internalChecked; |
Contributor
There was a problem hiding this comment.
Enforce exactly one visibility choice.
This currently accepts both checkboxes, which contradicts the template's "Check one" instruction and leaves the PR in an ambiguous state for release-note/customer-impact classification.
Suggested fix
const externalChecked = /- \[x\] External/i.test(impactContent);
const internalChecked = /- \[x\] Internal/i.test(impactContent);
- const hasVisibility = externalChecked || internalChecked;
+ const checkedCount = Number(externalChecked) + Number(internalChecked);
const errors = [];
@@
- if (!hasVisibility) {
+ if (checkedCount === 0) {
errors.push(
'❌ Missing visibility label.\n' +
'Check one of the boxes in the Visibility section:\n' +
'- [x] External — visible to customers\n' +
'- [x] Internal — not customer-facing'
);
}
+
+ if (checkedCount > 1) {
+ errors.push(
+ '❌ Ambiguous visibility label.\n' +
+ 'Check exactly one box in the Visibility section.'
+ );
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/ci-check-customer-impact.yml around lines 24 - 27, The
current visibility check (externalChecked, internalChecked, hasVisibility)
allows both boxes to be checked; change the validation to require exactly one
selection by replacing hasVisibility with a check using XOR semantics (e.g.,
const exactlyOneVisibility = externalChecked !== internalChecked) and treat any
other case (neither or both true) as a validation failure that fails the CI job
and emits a clear error message instructing the user to "Check one: External OR
Internal" (reference externalChecked, internalChecked, and the new
exactlyOneVisibility variable).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ci-check-customer-impact) that fails if the impact section is empty or no visibility checkbox is checkedCustomer Impact
This adds process tooling to improve how we document changes. No direct user-facing behavior change.
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit