Clarify how "Only impacted by breaking changes" interacts with workspace approval - #1736
Clarify how "Only impacted by breaking changes" interacts with workspace approval#1736logason wants to merge 1 commit into
Conversation
…ace approval Two corrections to the section added in #1733: - The setting was described as leaving the team out of the impacted list. It does not — the team still appears in the impacted stakeholders list (with per-item Breaking / Non-breaking labels) and Slack notifications are unaffected. It gates the required-reviewer decision only. - Adds the interaction with the workspace-level "Require approval from owning stakeholders", which customers ask about: the workspace setting decides who is pulled in (owners only, overriding their own notification setting), the team setting decides whether the change counts as impact, and both must pass. Also notes that for an owner team only owned items count towards the breaking/non-breaking decision. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThe guide now states that the breaking-only setting excludes teams from required review without removing them from the impacted stakeholder list. It also documents workspace-level approval requirements and owner-specific behavior. ChangesStakeholder review settings
Estimated code review effort: 2 (Simple) | ~5 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
pages/data-design/guides/managing-changes-impacting-multiple-stakeholders.mdxParsing error: Unexpected token (4:0) Comment |
📝 Changed routes:
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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
`@pages/data-design/guides/managing-changes-impacting-multiple-stakeholders.mdx`:
- Around line 108-110: Update the wording around the team setting to distinguish
impacted-stakeholder list membership from required-review eligibility: replace
unqualified claims that the setting determines whether a change counts as impact
with wording about required-review eligibility. Revise the statement around
“regardless of their individual settings” to clarify that workspace requirements
do not override a team’s “Only impacted by breaking changes” setting, including
the guidance in the section around the owner-team example.
- Around line 105-107: Revise the introductory “both say yes” sentence so it
applies only to impacted owner teams. Explicitly state that stakeholder-only
teams are governed by their own “When impacted as stakeholder” setting
regardless of the workspace approval setting, while preserving the existing
owner-team explanation.
🪄 Autofix
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 Plus
Run ID: d74a53de-6d78-45de-a05e-1c5a78db5051
📒 Files selected for processing (1)
pages/data-design/guides/managing-changes-impacting-multiple-stakeholders.mdx
| The workspace setting and the team setting answer different questions, and a team is only required to review when both say yes. | ||
|
|
||
| - **The workspace setting decides who is pulled in.** With it enabled, every impacted *owner* team is added as a required reviewer, regardless of what that team chose in its own notification settings. It does not apply to teams that are stakeholders but not owners – those still follow their own **When impacted as stakeholder** setting. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Scope the “both say yes” rule to owner teams.
Line 105 makes the workspace setting a prerequisite for every team. The workspace requirement applies only to impacted owner teams. A stakeholder-but-not-owner team follows its When impacted as stakeholder setting even when workspace approval is disabled. Rewrite the sentence to state this distinction explicitly.
Proposed wording
- The workspace setting and the team setting answer different questions, and a team is only required to review when both say yes.
+ For an impacted owner team, the workspace setting and the team setting must both allow required review. A stakeholder-but-not-owner team follows its `When impacted as stakeholder` setting.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| The workspace setting and the team setting answer different questions, and a team is only required to review when both say yes. | |
| - **The workspace setting decides who is pulled in.** With it enabled, every impacted *owner* team is added as a required reviewer, regardless of what that team chose in its own notification settings. It does not apply to teams that are stakeholders but not owners – those still follow their own **When impacted as stakeholder** setting. | |
| For an impacted owner team, the workspace setting and the team setting must both allow required review. A stakeholder-but-not-owner team follows its `When impacted as stakeholder` setting. | |
| - **The workspace setting decides who is pulled in.** With it enabled, every impacted *owner* team is added as a required reviewer, regardless of what that team chose in its own notification settings. It does not apply to teams that are stakeholders but not owners – those still follow their own **When impacted as stakeholder** setting. |
🤖 Prompt for 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.
In
`@pages/data-design/guides/managing-changes-impacting-multiple-stakeholders.mdx`
around lines 105 - 107, Revise the introductory “both say yes” sentence so it
applies only to impacted owner teams. Explicitly state that stakeholder-only
teams are governed by their own “When impacted as stakeholder” setting
regardless of the workspace approval setting, while preserving the existing
owner-team explanation.
| - **The team setting decides whether the change counts as impact in the first place.** If a team has *Only impacted by breaking changes* enabled and the branch only makes Non-breaking changes to its items, the team is not required to review – even under the workspace requirement. | ||
|
|
||
| In short: the workspace setting makes review the default for owning teams across all changes, and each team can narrow that to breaking changes only. Note that for an **owner** team, only the items it *owns* count towards that decision. A team that owns an item receiving a Non-breaking change, and is merely a stakeholder on another item receiving a Breaking one, is not required to review. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Keep impacted-list membership separate from required-review eligibility.
Line 108 says the team setting decides whether a change counts as impact in the first place. Line 100 says the team remains in the impacted stakeholders list. Replace the unqualified “impact” wording with “required-review eligibility.” Also update Lines 171-175, where “regardless of their individual settings” must not bypass Only impacted by breaking changes.
Proposed wording
- The team setting decides whether the change counts as impact in the first place.
+ For required-review eligibility, the team setting decides whether a change qualifies the team for required review.🤖 Prompt for 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.
In
`@pages/data-design/guides/managing-changes-impacting-multiple-stakeholders.mdx`
around lines 108 - 110, Update the wording around the team setting to
distinguish impacted-stakeholder list membership from required-review
eligibility: replace unqualified claims that the setting determines whether a
change counts as impact with wording about required-review eligibility. Revise
the statement around “regardless of their individual settings” to clarify that
workspace requirements do not override a team’s “Only impacted by breaking
changes” setting, including the guidance in the section around the owner-team
example.

Follow-up to #1733, prompted by a customer question: "If we have 'Require approval from owning stakeholders' enabled at a workspace level, how does the stakeholder setting 'Only impacted by breaking changes' work?"
Two changes
1. Fixes an inaccuracy in #1733. That PR described Only impacted by breaking changes as leaving the team out of the impacted list. It does not.
ignoreNonBreakingImpactsForReviewis only read byGetSpecificDomainRequiredReviewersUseCase— the impacted-stakeholders list still shows the team (GetImpactedDomainsUseCasedeliberately does not tier-filter, so the UI can render it),BranchBar__Dropdownadds per-item Breaking / Non-breaking labels for exactly these teams, andShouldSendDomainNotificationUseCasenever reads the setting, so Slack notifications are unaffected.2. Documents the interaction with the workspace setting, which the guide did not cover:
shouldRequireReviewfor impacted owner teams only, overriding their own notification setting. Stakeholder-but-not-owner teams still follow their own setting.passesNonBreakingFilteris ANDed after, for both the owner and stakeholder buckets, so it applies even under the workspace requirement.maxOwnerTier— only owned items count. A team owning a Non-breaking change and merely stakeholding a Breaking one is not required to review.Note for the product team
The in-product copy under the workspace toggle says "This overrides individual stakeholder review settings". That holds for the notification setting (a
#doNothingchoice is overridden for owners) but not for Only impacted by breaking changes, which still filters. Worth a follow-up ticket to narrow that sentence.yarn spellcheckpasses on the changed file.🤖 Generated with Claude Code
Summary by CodeRabbit