Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
97 changes: 97 additions & 0 deletions .github/workflows/breaking-change-check.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
name: Breaking change check

# Runs the breaking-change detector on PR opens, updates, AND on review
# submissions. Re-running on review submissions lets the script flip from
# red to green when a code-owner approves the PR — see the override logic
# in workspace/src/major-change-check.js.
#
# Lives in a dedicated workflow so review events don't re-trigger the full
# unit/e2e/type-diff suite.
on:
pull_request:
pull_request_review:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💬 Suggestion: The edited type fires when a review's body text is edited, not when its state changes. Editing a review comment doesn't change whether the PR is approved, so this trigger re-runs the full breaking-change check (clone, diff, scan) for zero functional effect. Each unnecessary run costs a full repo clone + schema diff scan. The submitted and dismissed types already cover all state-changing review actions.

Suggestion:

Suggested change
pull_request_review:
types: [submitted, dismissed]

types: [submitted, dismissed, edited]
merge_group:

concurrency:
group: shopify-cli-breaking-change-${{ github.event.pull_request.number || github.event.merge_group.head_sha || github.run_id }}
cancel-in-progress: true

env:
DEBUG: '1'
SHOPIFY_CLI_ENV: development
SHOPIFY_CONFIG: debug
PNPM_VERSION: '10.11.1'
BUNDLE_WITHOUT: 'test:development'
GH_TOKEN: ${{ secrets.SHOPIFY_GH_READ_CONTENT_TOKEN }}
GH_TOKEN_SHOP: ${{ secrets.SHOP_GH_READ_CONTENT_TOKEN }}
DEFAULT_NODE_VERSION: '24.1.0'
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Improvement: The breaking-change-check workflow pins DEFAULT_NODE_VERSION: '24.1.0' while tests-pr.yml, tests-main.yml, and tests-manual.yml all use '26.1.0'. This means the script runs on a different Node version than the rest of the CI suite. Likely an oversight from when the job was extracted from tests-pr.yml.

Suggestion:

Suggested change
DEFAULT_NODE_VERSION: '24.1.0'
DEFAULT_NODE_VERSION: '26.1.0'


jobs:
major-change-check:
# Skip fork PRs — fork tokens are read-only and the codeowner-override
# API calls would fail.
if: github.event.pull_request.head.repo.full_name == github.repository || github.event_name == 'merge_group'
name: 'Breaking change detection'
runs-on: ubuntu-latest
timeout-minutes: 30
permissions:
contents: read
pull-requests: write
steps:
- uses: actions/checkout@v3
with:
repository: ${{ github.event.pull_request.head.repo.full_name || github.event.repository.full_name }}
ref: ${{ github.event.pull_request.head.sha || github.event.merge_group.head_sha }}
# Full history so `git merge-base origin/<base> HEAD` (used by
# major-change-check.js to scope the diff to PR-only changes) can
# reach the divergence point. With fetch-depth: 1 the merge-base
# call fails and the script falls back to scanning everything,
# surfacing every pre-existing major changeset as "new".
fetch-depth: 0
- name: Setup deps
uses: ./.github/actions/setup-cli-deps
with:
node-version: ${{ env.DEFAULT_NODE_VERSION }}
# No build step: major-change-check.js operates on git-tracked
# source files (.ts, oclif.manifest.json, .changeset/*.md) and
# `git diff` output only — it never imports built artefacts. The
# script itself explicitly opts out of installing/building the
# baseline checkout for the same reason (see runMain in
# workspace/src/major-change-check.js). Skipping the build keeps
# approval-triggered re-runs fast (~30s instead of ~5–8min) so the
# check can flip green quickly when a code owner approves.
- name: Check for breaking changes
id: check
env:
# The default GITHUB_TOKEN can read PR reviews and the repo's
# CODEOWNERS file, which is all the override needs.
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
run: node workspace/src/major-change-check.js
- uses: marocchino/sticky-pull-request-comment@fcf6fe9e4a0409cd9316a5011435be0f3327f1e1 # v2.3.1
if: steps.check.outputs.has_breaking_changes == 'true'
with:
header: Breaking-change-detection
message: ${{ steps.check.outputs.report }}
recreate: true
# Override granted: breaking changes were detected but a code-owner
# approved, so we keep the sticky comment up to record the override
# footer ("✅ Override: approved by @user") emitted by the script.
- uses: marocchino/sticky-pull-request-comment@fcf6fe9e4a0409cd9316a5011435be0f3327f1e1 # v2.3.1
if: steps.check.outputs.has_breaking_changes != 'true' && steps.check.outputs.report != ''
with:
header: Breaking-change-detection
message: ${{ steps.check.outputs.report }}
recreate: true
# No breaking changes at all: delete any stale sticky comment from a
# previous push.
- uses: marocchino/sticky-pull-request-comment@fcf6fe9e4a0409cd9316a5011435be0f3327f1e1 # v2.3.1
if: steps.check.outputs.has_breaking_changes != 'true' && steps.check.outputs.report == ''
with:
header: Breaking-change-detection
delete: true
- name: Fail if breaking changes detected
if: steps.check.outputs.has_breaking_changes == 'true'
run: |
echo '::error::Breaking changes detected. See the sticky comment on the PR for details. A code-owner approval on this PR will turn this check green.'
exit 1
43 changes: 3 additions & 40 deletions .github/workflows/tests-pr.yml
Original file line number Diff line number Diff line change
Expand Up @@ -296,43 +296,6 @@ jobs:
message: ${{ steps.type-diff.outputs.report }}
recreate: true

major-change-check:
if: github.event.pull_request.head.repo.full_name == github.repository
name: 'Breaking change detection'
runs-on: ubuntu-latest
timeout-minutes: 30
outputs:
has_breaking_changes: ${{ steps.check.outputs.has_breaking_changes }}
steps:
- uses: actions/checkout@v3
with:
repository: ${{ github.event.pull_request.head.repo.full_name || github.event.repository.full_name }}
ref: ${{ github.event.pull_request.head.ref || github.event.merge_group.head_ref }}
# Full history so `git merge-base origin/<base> HEAD` (used by
# major-change-check.js) can reach the divergence point.
fetch-depth: 0
- name: Setup deps
uses: ./.github/actions/setup-cli-deps
with:
node-version: ${{ env.DEFAULT_NODE_VERSION }}
- name: Build
run: pnpm nx run-many --all --skip-nx-cache --target=build --output-style=stream
- name: Check for breaking changes
id: check
run: node workspace/src/major-change-check.js
- uses: marocchino/sticky-pull-request-comment@fcf6fe9e4a0409cd9316a5011435be0f3327f1e1 # v2.3.1
if: steps.check.outputs.has_breaking_changes == 'true'
with:
header: Breaking-change-detection
message: ${{ steps.check.outputs.report }}
recreate: true
- uses: marocchino/sticky-pull-request-comment@fcf6fe9e4a0409cd9316a5011435be0f3327f1e1 # v2.3.1
if: steps.check.outputs.has_breaking_changes != 'true'
with:
header: Breaking-change-detection
delete: true
- name: Fail if breaking changes detected
if: steps.check.outputs.has_breaking_changes == 'true'
run: |
echo '::error::Breaking changes detected. See the sticky comment on the PR for details.'
exit 1
# The breaking-change check moved to .github/workflows/breaking-change-check.yml
# so it can re-run on `pull_request_review` events without re-triggering
# the rest of this workflow.
Loading
Loading