👷 [RUM-15055] fix release/publish pipeline issues from v0.1.0#77
Conversation
- insert new changelog section after header, before existing entries - create PR with empty body instead of changelog content
bcaudan
changed the title
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0c009b9270
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| printLog('Creating GitHub PR...'); | ||
| return command`gh pr create --title v${newVersion} --body ${editedSection} --base main`.run().trim(); | ||
| return command`gh pr create --title v${newVersion} --body ${''} --base main`.run().trim(); |
There was a problem hiding this comment.
This seems a bit strange: ${''} probably harmless, but should be simply ''
There was a problem hiding this comment.
But then again, why are we putting it to an empty string now ? This will create an empty PR, is that intended now ?
There was a problem hiding this comment.
${''} is needed here — the command helper doesn't do shell parsing, so '' in the template string would be passed as a literal two-character argument. ${''} is how you inject an empty string.
Empty body is intentional — the changelog content would be duplicated and a pain to keep in sync with CHANGELOG.md.
| #### 3. Trigger the publish workflow | ||
| If you push fixup commits, move the tag to the latest commit: | ||
|
|
||
| After the PR is merged: | ||
| ```sh | ||
| git tag -a -f vX.Y.Z -m "vX.Y.Z" | ||
| git push -f origin vX.Y.Z | ||
| ``` |
There was a problem hiding this comment.
This seems like a very important step that is easily missed with the current wording. This in my opinion should be a warning highlighting that the release tag is created before the PR is merged, and that it's mandatory that the tag is moved to the latest commit before merging to prevent changes from being missed.
There was a problem hiding this comment.
Added a blockquote warning in the docs and a ⚠️ WARNING block in the script's terminal output right after the PR is created.
| #### 3. Trigger the publish workflow | ||
|
|
||
| A Slack message in `#rum-electron-sdk-ops` will include a link to the GitHub Actions publish workflow (sent when the tag is pushed in step 1). |
There was a problem hiding this comment.
This can be problematic no ? The current message states:
- 'MESSAGE=":rocket: electron-sdk ${CI_COMMIT_TAG} is ready to publish. Run the <${WORKFLOW_URL}|publish workflow> from \`${CI_COMMIT_TAG}\`"'
Ideally this should be sent when the PR is reviewed and merged, not when the tag is created, given that it's now created before anything else. If not possible at least we should change the wording to minimize the mistakes.
There was a problem hiding this comment.
Updated the message wording to better reflect the flow — the tag is pushed before the PR is merged. We could revisit the workflow trigger later if needed.
- make tag-before-merge warning prominent in script output and docs - reword Slack notification to reflect PR-review-first flow - update docs Slack message description to match new wording
2 participants
Motivation
Several bugs were discovered during and after the v0.1.0 release:
prepare-releaseinserted the new changelog section at the wrong position and used the changelog as the PR bodytag-release.ymlran asgithub-actions[bot], which lacks maintain permission to push tagsnotify-release-readyjob crashed due to unescaped backticks causing bash command substitutionpublish.ymlvalidation compared commit messages againstvX.Y.Zexactly, but GitHub squash-merge appends the PR number (v0.1.0 (#63))Changes
prepare-releaseyarn release(maintainer's machine) — removes the broken bot-driven auto-tagginggithub.ref_name) instead of the commit message — robust against squash merges and fixup commitsdry_runmode topublish.ymlto validate the full pipeline without publishingTest instructions
Trigger
publish.ymlwithdry_run=truefrom a release tag — validated against av0.0.0test tag from this branch (build passed, publish/release steps skipped as expected).Checklist