Skip to content

Remove unused 'Breaking change approval' job#7468

Draft
alfonso-noriega wants to merge 1 commit intofix-breaking-change-check-baselinefrom
remove-fake-approval-gate
Draft

Remove unused 'Breaking change approval' job#7468
alfonso-noriega wants to merge 1 commit intofix-breaking-change-check-baselinefrom
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

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

This stack of pull requests is managed by Graphite. Learn more about stacking.

@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
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.

1 participant