Skip to content

Remove unused 'Breaking change approval' job#7468

Merged
alfonso-noriega merged 2 commits into
mainfrom
remove-fake-approval-gate
May 14, 2026
Merged

Remove unused 'Breaking change approval' job#7468
alfonso-noriega merged 2 commits into
mainfrom
remove-fake-approval-gate

Conversation

@alfonso-noriega
Copy link
Copy Markdown
Contributor

@alfonso-noriega alfonso-noriega commented May 5, 2026

WHY are these changes introduced?

tests-pr.yml has two jobs for breaking changes:

  • major-change-check ("Breaking change detection") — runs the script and exit 1s on any detected breaking change.
  • major-change-approval ("Breaking change approval") — gated on the breaking-change-approval GitHub Environment, intended to require a @shopify/dev_experience member to approve before passing.

The second job's environment has no protection rules (gh api repos/Shopify/cli/environments → "protection_rules": []), so the "approval" job auto-passes in seconds with nobody actually approving anything. On PR #7466 the failure step ran at 09:46:20 and "Breaking change approval" turned green at 09:46:32 — 12s of "review", with zero human input. It's a placebo.

Worse, it's actively misleading: the script's failure message says "A member of @shopify/dev_experience must approve the breaking-change-approval environment", which sends people looking for a Settings → Environments → Required reviewers control that doesn't exist for them to use. PR #7469 (next in the stack) adds a real, working override — this PR removes the fake one so the two don't coexist.

WHAT is this pull request doing?

.github/workflows/tests-pr.yml

  • Removes the entire major-change-approval job.
  • Updates the failure-step error message to drop the dead reference to the environment.

workspace/src/major-change-check.js

  • Updates the sticky-comment report to remove the "this check will remain failed until a member of the team approves the workflow run" sentence — there is no workflow run to approve.

The breaking-change-approval GitHub Environment is left alone in repo settings. Other workflows might reference it, and deleting environments requires a separate API call. It's harmless to leave behind.

How to test your changes?

cd workspace
node --test src/major-change-check.test.js

All tests still pass — this PR is workflow + comment-text only, no logic changes.

In CI, the "Breaking change approval" check name will simply stop appearing on new PR runs. The "Breaking change detection" check still runs (and after PR #7469 will be unblockable via a code-owner approval).

Post-release steps

None.

Checklist

  • I've considered possible cross-platform impacts (Mac, Linux, Windows) — N/A
  • I've considered possible documentation changes
  • I've considered analytics changes to measure impact — N/A
  • The change is user-facing — N/A (internal CI tooling, no changeset needed)

Copy link
Copy Markdown
Contributor Author

alfonso-noriega commented May 5, 2026

@github-actions github-actions Bot added the no-changelog This PR doesn't include a changeset entry. Is an internal only change not relevant to end users. label May 5, 2026
@alfonso-noriega alfonso-noriega changed the base branch from fix-breaking-change-check-baseline to graphite-base/7468 May 6, 2026 14:17
@alfonso-noriega alfonso-noriega marked this pull request as ready for review May 12, 2026 10:50
@alfonso-noriega alfonso-noriega requested a review from a team as a code owner May 12, 2026 10:50
@alfonso-noriega alfonso-noriega force-pushed the remove-fake-approval-gate branch from 04411a5 to 15299d0 Compare May 14, 2026 10:55
@alfonso-noriega alfonso-noriega requested a review from a team as a code owner May 14, 2026 10:55
@graphite-app graphite-app Bot changed the base branch from graphite-base/7468 to main May 14, 2026 10:58
@alfonso-noriega alfonso-noriega force-pushed the remove-fake-approval-gate branch from 6e6bea7 to 8fdaa1f Compare May 14, 2026 10:58
@alfonso-noriega alfonso-noriega added this pull request to the merge queue May 14, 2026
Merged via the queue into main with commit bcb63d5 May 14, 2026
26 of 28 checks passed
@alfonso-noriega alfonso-noriega deleted the remove-fake-approval-gate branch May 14, 2026 13:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-changelog This PR doesn't include a changeset entry. Is an internal only change not relevant to end users.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants