-
Notifications
You must be signed in to change notification settings - Fork 56
chore: merge prerelease into publish-v2 job #1421
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
base: main
Are you sure you want to change the base?
Conversation
lewgordon-amplitude
left a comment
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.
LGTM!
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.
Pull request overview
This PR consolidates the pre-release and publish-v2 workflows into a single workflow, eliminating the need for NPM tokens by using OIDC authentication instead. Users can now trigger pre-releases through the publish-v2 workflow by selecting a release type from a dropdown menu.
Key Changes
- Added a
releaseTypeinput to the publish-v2 workflow with options: release, prerelease, and dry-run - Migrated the prerelease job from feature-branch-prerelease.yml into publish-v2.yml with OIDC support
- Deleted the standalone feature-branch-prerelease.yml workflow file
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
.github/workflows/publish-v2.yml |
Added releaseType input parameter, conditional logic to run deploy job only for releases, and new prerelease job that handles both prerelease and dry-run operations using OIDC authentication |
.github/workflows/feature-branch-prerelease.yml |
Removed entire workflow file as its functionality has been migrated to publish-v2.yml |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
bugbot run |
Co-authored-by: Copilot <[email protected]>
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.
Comment @cursor review or bugbot run to trigger another review on this PR
Mercy811
left a comment
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.
Should we only allow "release" from main branch?
|
@Mercy811 the trusted publishing policy will prevent the release from any branch other than 'main'. Which makes me realize, this isn't going to work (in it's current form at least) because pre-releases need to be run from a branch other than main, but Trusted Publishing only works on main |
|
@Mercy811 I just updated it so that you have to run pre-release from main, but it will check out the branch that you specify to be "pre-released". |
Mercy811
left a comment
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.
Thanks @daniel-graham-amplitude LGTM!
… no-ticket/oidc-pre-release
|
bugbot run |
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.
Comment @cursor review or bugbot run to trigger another review on this PR
| shell: bash | ||
| run: | | ||
| git config --global user.name amplitude-sdk-bot | ||
| git config --global user.email [email protected] |
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.
Bug: Git user changed from amplitude-sdk-dev to amplitude-sdk-bot
The composite action configures Git with amplitude-sdk-bot user, but the original publish-v2.yml deploy job used amplitude-sdk-dev (the removed lines show git config --global user.name amplitude-sdk-dev and email [email protected]). This means releases will now be attributed to a different GitHub account than before, which may affect commit attribution, permissions, or downstream systems that depend on the author identity.
| echo "branch=${{ github.event.inputs.branch }}" >> $GITHUB_OUTPUT | ||
| else | ||
| echo "branch=${{ github.ref_name }}" >> $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.
Bug: Prerelease defaults to main branch when unspecified
The branch input description says "Leave empty to use current branch," but due to the branch protection check on lines 29-35, the workflow can only run from main. When the branch input is empty, the fallback github.ref_name will always be "main". This means running a prerelease without specifying a branch creates versions like 1.0.0-main.0 from the main branch, which contradicts the job name "Prerelease feature branch" and is likely unintended behavior. Users expecting "current branch" to mean their feature branch will accidentally release from main.
Summary
Merged the pre-release and publish-v2 workflow into one job. This is so that pre-releases can be published without the need for NPM tokens.
To make a pre-release now, run publish-v2 and select "prerelease" from this dropdown
Checklist
Note
Consolidates prerelease into
publish-v2with new inputs and branch checks, replaces repeated steps with a reusablebuild-and-testcomposite action, and removes the old prerelease workflow./.github/workflows/publish-v2.ymlwithreleaseType(release/prerelease/dry-run) and optionalbranchinput.mainfor release.deployruns only forreleaseand usesPUBLISH_FROMinput; keeps lerna versioning and NPM publish.prereleasehandlesprerelease/dry-run, determines target branch, computesPREID, runs versioning and (for prerelease) publish from git./.github/workflows/feature-branch-prerelease.yml./.github/actions/build-and-test/action.ymlto cache deps, set up Node, install, build, test, lint, set Git user, and configure AWS credentials.Written by Cursor Bugbot for commit 81df507. Configure here.