-
Notifications
You must be signed in to change notification settings - Fork 91
ci: fix next-release with git tag #1577
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
WalkthroughThis PR updates the publishing scripts in Changes
Possibly related PRs
Suggested reviewers
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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: 0
🧹 Nitpick comments (2)
package.json (2)
28-28: Refactor: Optimize repeated command substitution for Git hash.
The updated"publish-manual-prerelease"script correctly appends the Git commit's short hash for dynamic pre-release versioning. As a refinement, consider storing the result of$(git rev-parse --short HEAD)in an environment variable to avoid invoking the command twice. This change would improve readability and execution efficiency, especially if the command’s output needs to be consistent across multiple options.For example, you could refactor as follows:
-"lerna publish prerelease --no-git-tag-version --no-push --preid next.$(git rev-parse --short HEAD) --dist-tag next.$(git rev-parse --short HEAD)" +"GIT_HASH=$(git rev-parse --short HEAD) && lerna publish prerelease --no-git-tag-version --no-push --preid next.$GIT_HASH --dist-tag next.$GIT_HASH"
29-29: Review: Validate FORCE_PUBLISH usage and dynamic Git hash integration.
The"publish-prerelease"script correctly calculatesFORCE_PUBLISHand integrates the dynamic pre-release identifier using the Git hash. Similar to the previous script, consider reusing the computed Git hash by storing it in an environment variable to avoid redundant command calls. Additionally, ensure that echoing theFORCE_PUBLISHvariable is intended for logging purposes and does not expose sensitive information.A possible refactor option could be:
-FORCE_PUBLISH=$(lerna changed --json | jq '. | map(.name) | join (\",\")' -r) && echo $FORCE_PUBLISH && lerna publish prerelease --no-git-tag-version --no-push --preid next.$(git rev-parse --short HEAD) --dist-tag next.$(git rev-parse --short HEAD) --yes --force-publish=${FORCE_PUBLISH} +GIT_HASH=$(git rev-parse --short HEAD) && \ +FORCE_PUBLISH=$(lerna changed --json | jq '. | map(.name) | join (\",\")' -r) && \ +echo $FORCE_PUBLISH && \ +lerna publish prerelease --no-git-tag-version --no-push --preid next.$GIT_HASH --dist-tag next.$GIT_HASH --yes --force-publish=${FORCE_PUBLISH}This approach minimizes redundant git calls and improves maintainability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
package.json(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build-and-test
This is a fixup for #1576
It is related to #1503 and #1442
Issue
In #1576, I introduced the
--canaryoption, which appends the Git hash to prerelease tags to prevent duplicate versions.This worked well on my local setup (when testing the
yarn run publish-prereleasecommand), but unfortunately, something goes wrong when it runs in the CI, and I have not been able to find out why.For some reason, the version created on the CI is wrong. For instance, the
@requestnetwork/data-accesspackage was published with version0.23.1-next.783+2b7c9697(see here in the CI) instead of0.44.1-next.2132+2b7c9697(see previous PR description where the command was run on my local env). The version number is completely off and shows the same behavior we suffered in #1442.It seems related to lerna/lerna#2622. Unfortunately, browsing this issue does not disclose any viable solution for now. Perhaps updating Lerna could work (not tested), but that's for another time. I have found another workaround for now.
Workaround
The workaround is to revert #1576 and instead use the
gitcommand to append the Git hash to the version tag.I have tested the command in the CI (see the first commit of this PR), and it works properly.
The output version will be slightly different:
0.44.1-next.21e95031.0, but I find it acceptable.Let me know what you think.
Summary by CodeRabbit