ci: fix prelease flag set wrong if releasing from a merge#37258
Conversation
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
|
WalkthroughA refactoring of prerelease state handling in the release publishing workflow that introduces an intermediate variable Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes The change involves variable state refactoring with cascading effects on downstream conditional logic. While localized to one file, understanding the impact on Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/release-action/src/publishRelease.ts (1)
67-67: Pre-existing bug: Missing template literal for version interpolation.The error message doesn't interpolate the
newVersionvariable. This is a pre-existing issue not introduced by this PR.Apply this diff to fix:
- throw new Error('Could not find changelog entry for version newVersion'); + throw new Error(`Could not find changelog entry for version ${newVersion}`);
🧹 Nitpick comments (1)
packages/release-action/src/publishRelease.ts (1)
39-46: LGTM! The prerelease flag logic is now correctly implemented.The refactoring correctly addresses the issue where the prerelease flag was set incorrectly when releasing from a merge. By introducing
startAsPreReleaseto capture the initial state and deriving the finalprereleasevariable as!mergeFinal && startAsPreRelease, the logic now ensures:
- When
mergeFinalis true (releasing from a merge),prereleaseis false even if it started as a prerelease- When
mergeFinalis false and it started as a prerelease,prereleaseremains trueThis correctly propagates to line 123 where the GitHub release is created.
Optional: Consider adding a clarifying comment.
To improve maintainability, you might add a brief comment explaining the derived logic:
+ // prerelease is true only if we started as prerelease AND are not merging final const prerelease = !mergeFinal && startAsPreRelease;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/release-action/src/publishRelease.ts(1 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). (3)
- GitHub Check: 📦 Build Packages
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #37258 +/- ##
===========================================
- Coverage 67.59% 67.58% -0.01%
===========================================
Files 3341 3341
Lines 114016 114016
Branches 20665 20665
===========================================
- Hits 77068 77059 -9
- Misses 34267 34277 +10
+ Partials 2681 2680 -1
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
Proposed changes (including videos or screenshots)
Issue(s)
Steps to test or reproduce
Further comments
Summary by CodeRabbit