-
-
Notifications
You must be signed in to change notification settings - Fork 214
ci: Fix auto-release #1074
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
ci: Fix auto-release #1074
Conversation
|
🚀 Thanks for opening this pull request! |
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughSwitches non-root package change detection in Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes
Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1074 +/- ##
==========================================
+ Coverage 43.46% 43.49% +0.02%
==========================================
Files 61 61
Lines 3587 3587
==========================================
+ Hits 1559 1560 +1
+ Misses 2028 2027 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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)
release.config.js(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). (5)
- GitHub Check: Test Dart 3.10, Windows
- GitHub Check: Test Flutter 3.38, Windows
- GitHub Check: Test Flutter beta
- GitHub Check: Test Flutter 3.38, macOS
- GitHub Check: Test Flutter 3.38, Ubuntu
🔇 Additional comments (1)
release.config.js (1)
88-88: Tag-based change detection logic for non-root packages looks correct, but double-check CI behavior on failure.The new
verifyConditionsCmdcorrectly:
- Uses the last
${packageName}-*tag when present and diffs from that tag toHEADfor path-based detection.- Falls back to scanning the full history for
packages/${packageName}/when no such tag exists, enabling an initial release.This achieves the intended behavior of skipping a release when the package directory hasn’t changed since its last tagged release. The quoting and interpolation of
packageNamein the command also look robust.One thing to verify in your CI pipeline: an
exit 1fromverifyConditionswill surface as a failure from the@semantic-release/execplugin, so ensure your workflow is explicitly treating “no changes, skipping release” as a non-problematic outcome (e.g., by how you orchestrate multi‑package runs or interpret job status).
release.config.js
Outdated
| releasedLabels: ['state:released<%= nextRelease.channel ? `-\${nextRelease.channel}` : "" %>'], | ||
| failComment: false, | ||
| failTitle: false |
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.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, let's find and examine the release.config.js file
fd -type f -name "release.config.js" | head -5Repository: parse-community/Parse-SDK-Flutter
Length of output: 210
🌐 Web query:
EJS template literal interpolation escaped dollar sign behavior
💡 Result:
Short answer: escape it the same way you do in JavaScript template literals — prefix the $ with a backslash. Use ${} to render a literal "${...}" instead of starting interpolation. [1][2]
Notes:
- If the template literal is inside an EJS expression that itself is parsed by EJS, make sure the backslash survives EJS output (you may need to double-escape in the EJS source so the rendered client/server JS gets a single backslash). Example:
- In EJS source: <%= "
Price is \\${price}" %> → renders:Price is ${price}in the generated HTML/JS.
- In EJS source: <%= "
- EJS’s tags (<%= / <%- / <% %>) control whether EJS escapes HTML; the template-literal escaping behavior is pure JavaScript semantics after EJS has produced the JS string. [3][1]
Sources:
[1] MDN — Template literals (escaping
[2] StackOverflow — escaping $ in template literals ($ or ${}).
[3] EJS docs/npm — EJS output tags and behavior.
Based on my verification through EJS and JavaScript template literal documentation, the original review comment is correct.
Here's what happens:
- In the EJS expression
<%= nextRelease.channel ?-${nextRelease.channel}: "" %>, the backtick string-\${nextRelease.channel}is JavaScript code. - When EJS evaluates this expression, the
\${}escape sequence is a JavaScript template literal escape, not an EJS escape. - In JavaScript template literals,
\${}renders as a literal${}string, preventing interpolation. - Therefore, the output will be
state:released-${nextRelease.channel}(literal text, not the actual channel value).
The suggested fix using string concatenation is the correct approach.
The releasedLabels EJS expression produces a literal ${nextRelease.channel} instead of the actual channel value.
The backtick string -\${nextRelease.channel} uses JavaScript's template literal escape sequence \${}, which renders as a literal string ${} rather than interpolating the value. This means you'll get state:released-${nextRelease.channel} instead of state:released-beta for beta releases.
Use string concatenation instead:
- releasedLabels: ['state:released<%= nextRelease.channel ? `-\${nextRelease.channel}` : "" %>'],
+ releasedLabels: ['state:released<%= nextRelease.channel ? "-" + nextRelease.channel : "" %>'],
failComment: false,
failTitle: falseThis will correctly yield state:released for stable releases and state:released-beta, state:released-next-major, etc., for channelled releases.
📝 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.
| releasedLabels: ['state:released<%= nextRelease.channel ? `-\${nextRelease.channel}` : "" %>'], | |
| failComment: false, | |
| failTitle: false | |
| releasedLabels: ['state:released<%= nextRelease.channel ? "-" + nextRelease.channel : "" %>'], | |
| failComment: false, | |
| failTitle: false |
🤖 Prompt for AI Agents
In release.config.js around lines 123-125, the EJS expression for releasedLabels
uses a backticked string with an escaped ${...} which renders the literal
`${nextRelease.channel}` instead of interpolating; replace that EJS expression
to build the label via string concatenation so it produces "state:released" for
no channel and "state:released-<channel>" when nextRelease.channel is set (e.g.
use "state:released" + (nextRelease.channel ? "-" + nextRelease.channel : "")
inside the <%= %>), keeping the array structure and the existing
failComment/failTitle settings unchanged.
|
🎉 This change has been released in version 9.3.0 |
Summary by CodeRabbit
Chores
Notes
✏️ Tip: You can customize this high-level summary in your review settings.