-
-
Notifications
You must be signed in to change notification settings - Fork 3.5k
ci(release): do not auto-merge major releases #9710
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
WalkthroughAdds steps to detect "major" changesets in CI: new major-detection steps in PR and release workflows that set an output flag, and the release workflow's Changesets auto-merge step is gated to only run when no major changeset is found. Also updates build doc noting a dependency major bump. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant GH as GitHub Actions
participant Release as release.yml
participant PRjob as pr.yml Provenance
participant MajorCheck as Major-check Step
participant Changesets as Changesets Run
participant AutoMerge as Auto-merge Changesets
GH->>PRjob: Run PR workflow (provenance)
PRjob->>MajorCheck: Scan `.changeset/*.md` for "major"
MajorCheck-->>PRjob: output found = true/false
PRjob->>GH: Echo found
GH->>Release: Run release workflow
Release->>MajorCheck: Scan `.changeset/*.md` for "major"
MajorCheck-->>Release: output found = true/false
Release->>Changesets: Run Changesets
alt found == false
Release->>AutoMerge: Condition ok -> Auto-merge
AutoMerge-->>GH: Merge performed
else found == true
Release--x AutoMerge: Auto-merge skipped (major present)
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
Comment |
View your CI Pipeline Execution ↗ for commit 8e2a348
☁️ Nx Cloud last updated this comment at |
Sizes for commit 8e2a348:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/release.yml
(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Preview
- GitHub Check: Test
.github/workflows/release.yml
Outdated
for file in ".changeset/*.md"; do | ||
if [[ $(cat $file) =~ $regex ]]; then | ||
echo "found=true" >> $GITHUB_OUTPUT | ||
fi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix the glob so major changesets are actually detected.
With the loop written as for file in ".changeset/*.md"; do
, the shell never expands the wildcard—the body runs once against the literal string .changeset/*.md
. That means cat
fails, found
never flips to true
, and majors are still auto-merged. Please drop the quotes (and guard the read) so real files are inspected.
- for file in ".changeset/*.md"; do
- if [[ $(cat $file) =~ $regex ]]; then
- echo "found=true" >> $GITHUB_OUTPUT
- fi
- done
+ shopt -s nullglob
+ for file in .changeset/*.md; do
+ if [[ -f "$file" && $(<"$file") =~ $regex ]]; then
+ echo "found=true" >> "$GITHUB_OUTPUT"
+ break
+ fi
+ done
📝 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.
for file in ".changeset/*.md"; do | |
if [[ $(cat $file) =~ $regex ]]; then | |
echo "found=true" >> $GITHUB_OUTPUT | |
fi | |
shopt -s nullglob | |
for file in .changeset/*.md; do | |
if [[ -f "$file" && $(<"$file") =~ $regex ]]; then | |
echo "found=true" >> "$GITHUB_OUTPUT" | |
break | |
fi | |
done |
🤖 Prompt for AI Agents
.github/workflows/release.yml lines 45-48: the for-loop uses a quoted glob ("
.changeset/*.md") so the shell doesn't expand it and the loop iterates once over
the literal string; remove the quotes so the glob expands (for file in
.changeset/*.md; do) and add a guard to ensure the path is a real file before
reading (e.g., if [ -f "$file" ]; then ... fi), also quote the $file when
passing to cat or other commands to handle spaces.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/pr.yml
(1 hunks).github/workflows/release.yml
(2 hunks)grumpy-plants-build.md
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- grumpy-plants-build.md
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/release.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Test
.github/workflows/pr.yml
Outdated
- name: Check for Changesets marked as major | ||
id: major | ||
run: | | ||
echo "found=false" >> $GITHUB_OUTPUT | ||
regex="(major)" | ||
shopt -s nullglob | ||
for file in .changeset/*.md; do | ||
if [[ $(cat $file) =~ $regex ]]; then | ||
echo "found=true" >> $GITHUB_OUTPUT | ||
fi | ||
done |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Restrict major detection to the changeset frontmatter
Scanning the entire markdown body for the literal "major"
will flag perfectly valid non-major changesets whose description happens to use that word (e.g. “Major docs cleanup”), which in turn blocks the auto-merge workflow unnecessarily. We should only examine the frontmatter bump declarations when deciding if a changeset is major. Example adjustment:
- name: Check for Changesets marked as major
id: major
run: |
echo "found=false" >> $GITHUB_OUTPUT
shopt -s nullglob
for file in .changeset/*.md; do
- if [[ $(cat $file) =~ $regex ]]; then
- echo "found=true" >> $GITHUB_OUTPUT
- fi
+ if awk 'BEGIN { fm=0; found=0 }
+ /^---$/ { fm++; if (fm >= 2) exit }
+ fm == 1 && $0 ~ /:\s*(\"|\047)?major(\"|\047)?\s*$/ { exit 0 }
+ END { exit 1 }
+ ' "$file"; then
+ echo "found=true" >> $GITHUB_OUTPUT
+ break
+ fi
done
This keeps the step lightweight while avoiding false positives by checking only the release bump definitions between the ---
delimiters.
📝 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.
- name: Check for Changesets marked as major | |
id: major | |
run: | | |
echo "found=false" >> $GITHUB_OUTPUT | |
regex="(major)" | |
shopt -s nullglob | |
for file in .changeset/*.md; do | |
if [[ $(cat $file) =~ $regex ]]; then | |
echo "found=true" >> $GITHUB_OUTPUT | |
fi | |
done | |
- name: Check for Changesets marked as major | |
id: major | |
run: | | |
echo "found=false" >> $GITHUB_OUTPUT | |
regex="(major)" | |
shopt -s nullglob | |
for file in .changeset/*.md; do | |
if awk 'BEGIN { fm=0; found=0 } | |
/^---$/ { fm++; if (fm >= 2) exit } | |
fm == 1 && $0 ~ /:\s*(\"|\047)?major(\"|\047)?\s*$/ { exit 0 } | |
END { exit 1 } | |
' "$file"; then | |
echo "found=true" >> $GITHUB_OUTPUT | |
break | |
fi | |
done |
🤖 Prompt for AI Agents
In .github/workflows/pr.yml around lines 87 to 97, the job currently scans
entire changeset markdown for the literal "major" causing false positives;
change the loop to read only the frontmatter (the content between the first pair
of '---' delimiters) for each .changeset/*.md and test that substring for the
bump type "major" (or the specific frontmatter key/value used for bumps) before
setting found=true; keep the same lightweight shell loop and nullglob but
replace the body so it extracts frontmatter only and checks within that scope
for "major".
🎯 Changes
If a changeset is marked to release a major, it should not run the auto-merge logic.
Inspired by https://github.com/withastro/astro/blob/main/.github/workflows/check-merge.yml
✅ Checklist
pnpm test:pr
.🚀 Release Impact
Summary by CodeRabbit
Chores
Documentation