Conversation
.github/workflows/new-release.yml
Outdated
| script: | | ||
| const version = process.env.APP_VERSION; | ||
| const [owner, repo] = process.env.GITHUB_REPOSITORY.split("/"); | ||
| const title = `Bump version to v${version}`; |
There was a problem hiding this comment.
Would it be useful for the title to be something like Bump version to v${version} MERGE NOW to remind devs that this is the PR we merge immediately?
There was a problem hiding this comment.
good call, I'll add it
| } catch (error) { | ||
| if (error.status === 422) { | ||
| core.warning(`Bump version pull request could not be created automatically: ${error.message}`); | ||
| const { data: existing } = await github.rest.pulls.list({ |
There was a problem hiding this comment.
just curious - in the event of 422, should we be creating this PR manually or just re-running the pipeline?
There was a problem hiding this comment.
I think it'll depend on the error message. If it indicates something intermittent is happening we could try deleting the release branch and running the pipeline again. If it indicates something that we could fix in our pipeline, then we try fixing it before running it again. If it's something out of our control or too hairy then maybe would be worth manually creating the PR while we figure things out.
But as discussed on the other thread, having the PR open is not mission critical, so as long as we have the branch there we should be able to issue and distribute the builds. :)
| run: | | ||
| git push --set-upstream origin bump-version | ||
|
|
||
| - name: Create bump-version pull request |
There was a problem hiding this comment.
Do we need to create a PR for the release branch? Your description on slack mentioned a second PR, but maybe we don't need it if we're just pushing the deploy/the tag from this branch and deleting it after
There was a problem hiding this comment.
Yeah so the intent of the release PR here is to be mostly informational so we expose to the team what is being released on the given version, but you are right that technically speaking we don't need to open it to submit the release. Wdyt?
There was a problem hiding this comment.
That makes sense. No need to open a PR 👍
There was a problem hiding this comment.
adding @aristidesstaffieri to the loop as he had the same question about opening or not a PR
I personally think it's still worth opening the PR so we have a clear way of exposing to the team what is being released, so people can easily review it specially any QA fix added and add comments/questions to it. In case we don't open the PR I think we'll lose value in terms of communication.
How do you guys feel about it? Would you have some alternative suggestion for enabling people to review/comment on release candidate changes?
Thanks!
There was a problem hiding this comment.
Is there a difference between the branch that the action uses and the emergency branch it creates?
To me the downside of this is that this flow doesn't retain the history of the work flow in git, so the PR serves as a good way for folks to see the changes but then that PR is not merged to main and we lose the historical context of the group of work(from the git log perspective).
It does feel ceremonial to open a PR we never plan to merge for the purposes of temporary visibility, especially if meaningful conversations happen in that PR but will not be easily referenced in the future if someone wants to review what happened.
There was a problem hiding this comment.
Is there a difference between the branch that the action uses and the emergency branch it creates?
humm good point, I just realized now that the "Use workflow from" dropdown also allow us to select a tag which means we could probably simplify this workflow by removing the "Branch this release from" input 🤔
But I can still see some advantage of having both "Use workflow from" and "Branch this release from" inputs as the first input would allow us to select any workflow file from any branch/tag whereas the second input will tell the workflow which branch/tag to use as reference to create the release branches, etc. If we use only 1 input for everything this means we wouldn't be able to use a more recent workflow file while still referencing an old release tag to issue an emergency release.
That being said, I'm not sure this advantage I mentioned is worth the complexity of having 2 separate inputs specially considering the second input is a text input which is not the best UX ever when comparing to having only 1 dropdown. If we need to use a more recent workflow file with an old tag we can always branch-off from the tag and update the workflow file there.
Wdyt?
especially if meaningful conversations happen in that PR but will not be easily referenced in the future if someone wants to review what happened.
Nice call. I think an approach here would be adding the Release PR reference on the GH Release Notes, so we can easily track conversations related to that specific release. => this would still require some manual work though to paste the PR reference on the GH Release notes.
I just had another idea now to automate the release footprint:
- We have the release workflow create a third branch named
release - The
release/v{version}oremergency-release/v{version}branch is created based on thereleasebranch - After QA is done and all fixes are in, we
do merge the Release PRon thereleasebranch - We push the tag from the
releasebranchafter the Release PR is merged - Every time we run the release workflow it deletes the existing
releasebranch (if any) and creates a fresh one, which means thereleasebranch is ephemeral so we don't need to worry about having to update or maintain it - BUT, since the
release tagis not ephemeral this means we now have the entire release footprint including the Release PR content on git and we can easily get back to it through our release tags
How does that sound?
There was a problem hiding this comment.
👆 Adding to the above idea: considering we'd have a predictable release branch we could have another workflow dedicated to monitor when a release PR is merged to the release branch and then automatically push the release tag and trigger the iOS and Android prod build workflows. So this means once we merge the release PR it would automatically push the tag and generate the binaries we will use to submit for Apple Store and Google Play review. :)
There was a problem hiding this comment.
FYI, I'll try the proposal above with bump-version, release and release/v1.11.24 branches to see how it goes in this release. I'll do it manually this time to use as an example, then we can work on the GH Actions improvements after we decide which git flow we'd like to automate.
There was a problem hiding this comment.
bump-versionbranch and PR: Bump version to v1.11.24 [merge now] #675releasebranch: https://github.com/stellar/freighter-mobile/tree/releaserelease/v1.11.24branch and PR: v1.11.24 #677
There was a problem hiding this comment.
Pull request overview
This PR enhances the release workflow to support both regular releases from main and emergency releases from previous tags. It automates version bumping on the main branch via a separate PR and provides better visibility for emergency releases through distinct branch naming and PR titles.
Changes:
- Added
branch_frominput parameter to allow branching from previous release tags for emergency releases - Introduced automatic creation of a "bump-version" PR to update version numbers on main after initiating any release
- Implemented validation for branch/tag inputs with helpful error messages showing available tags
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 11 comments.
| File | Description |
|---|---|
| .github/workflows/new-release.yml | Enhanced release workflow with emergency release support, version validation, base branch management for tag-based releases, and automatic bump-version PR creation |
| .github/actions/validate-branch-from/action.yml | New composite action that validates branch_from input, normalizes tag formats, and displays available tags on validation failure |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # For tags, create a base branch name from the tag | ||
| # GitHub requires PR base to be a branch, not a tag | ||
| BASE_BRANCH="base-${BRANCH_FROM}" |
There was a problem hiding this comment.
The base branch name pattern "base-{tag}" could conflict with existing branches or create confusion. For example, if someone creates an emergency release from v1.15.16, it creates "base-v1.15.16". If another emergency release needs to be created from the same tag later, this branch already exists and may have diverged.
Consider using a more unique naming pattern that includes a timestamp or the new version, e.g., "emergency-base-{tag}-to-v{new_version}" or using the existing base branch only if it hasn't diverged from the tag.
| # For tags, create a base branch name from the tag | |
| # GitHub requires PR base to be a branch, not a tag | |
| BASE_BRANCH="base-${BRANCH_FROM}" | |
| # For tags, create a unique, descriptive base branch from the tag and target version | |
| # GitHub requires PR base to be a branch, not a tag | |
| BASE_BRANCH="emergency-base-${BRANCH_FROM}-to-v${APP_VERSION}" |
| echo "Available tags:" | ||
| echo "$TAGS" | while read tag; do | ||
| echo " - $tag" | ||
| done |
There was a problem hiding this comment.
The 'read' command in the while loop might not work correctly if TAGS is empty. When there are no tags, the loop will still execute once with an empty value, potentially causing unnecessary output.
Consider adding a check to verify TAGS is not empty before the loop, or use a more robust iteration method like "while IFS= read -r tag".
| } catch (error) { | ||
| if (error.status === 422) { | ||
| core.warning(`Pull request could not be created automatically: ${error.message}`); | ||
| const { data: existing } = await github.rest.pulls.list({ | ||
| owner, | ||
| repo, | ||
| head: `${owner}:${branch}`, | ||
| base: baseBranch, | ||
| state: "open" | ||
| }); | ||
|
|
||
| if (existing.length > 0) { | ||
| await setOutputs(existing[0]); | ||
| } else { | ||
| throw error; | ||
| } | ||
| } else { | ||
| throw error; | ||
| } | ||
| } | ||
|
|
||
| - name: Create bump-version branch | ||
| run: | | ||
| git checkout main | ||
| git pull origin main | ||
| # Delete bump-version branch if it exists locally | ||
| git branch -D bump-version 2>/dev/null || true | ||
| # Delete bump-version branch if it exists remotely | ||
| git push origin --delete bump-version 2>/dev/null || true | ||
| git checkout -b bump-version | ||
|
|
||
| - name: Update app version on bump-version branch | ||
| run: | | ||
| APP_VERSION="${{ steps.normalize_version.outputs.app_version }}" | ||
| node ./scripts/set-app-version "$APP_VERSION" | ||
|
|
||
| - name: Commit version bump on bump-version branch | ||
| run: | | ||
| APP_VERSION="${{ steps.normalize_version.outputs.app_version }}" | ||
| git status | ||
| if [ -z "$(git status --porcelain)" ]; then | ||
| echo "No changes detected after running set-app-version." | ||
| exit 1 | ||
| fi | ||
| git add . | ||
| git commit -m "chore: bump app version to v${APP_VERSION}" | ||
|
|
||
| - name: Push bump-version branch | ||
| run: | | ||
| git push --set-upstream origin bump-version | ||
|
|
||
| - name: Create bump-version pull request | ||
| uses: actions/github-script@v8 | ||
| env: | ||
| APP_VERSION: ${{ steps.normalize_version.outputs.app_version }} | ||
| with: | ||
| script: | | ||
| const version = process.env.APP_VERSION; | ||
| const [owner, repo] = process.env.GITHUB_REPOSITORY.split("/"); | ||
| const title = `Bump version to v${version}`; | ||
| const body = `Bumping version on \`main\` branch to \`v${version}\`.`; | ||
|
|
||
| async function setOutputs(pr) { | ||
| core.notice(`Bump version pull request ready: ${pr.html_url}`); | ||
| core.setOutput("bump_version_pr_number", pr.number.toString()); | ||
| core.setOutput("bump_version_pr_url", pr.html_url); | ||
| } | ||
|
|
||
| try { | ||
| const { data } = await github.rest.pulls.create({ | ||
| owner, | ||
| repo, | ||
| head: "bump-version", | ||
| base: "main", | ||
| title, | ||
| body | ||
| }); | ||
| await setOutputs(data); | ||
| } catch (error) { | ||
| if (error.status === 422) { | ||
| core.warning(`Bump version pull request could not be created automatically: ${error.message}`); | ||
| const { data: existing } = await github.rest.pulls.list({ | ||
| owner, | ||
| repo, | ||
| head: `${owner}:bump-version`, | ||
| base: "main", | ||
| state: "open" | ||
| }); | ||
|
|
||
| if (existing.length > 0) { | ||
| await setOutputs(existing[0]); | ||
| } else { | ||
| throw error; | ||
| } | ||
| } else { | ||
| throw error; | ||
| } | ||
| } |
There was a problem hiding this comment.
The error handling for the release PR creation (lines 236-255) catches 422 errors and attempts to find existing PRs, but the error handling for the bump-version PR (lines 314-333) has the same logic. However, both use different approaches for handling the "no existing PR" case.
For the release PR, if no existing PR is found, it re-throws the error. For bump-version PR, it also re-throws. This is consistent, but if a PR already exists for the release branch with a different base (e.g., someone manually created it against 'main' instead of the base branch), the workflow will silently succeed without notifying about the base branch mismatch.
Consider adding validation that the found PR has the expected base branch.
| OUTPUT_DELIM="__RELEASE_DESCRIPTION__" | ||
|
|
||
| # For emergency releases (not from main), use simpler release notes | ||
| if [ "$BRANCH_FROM" != "main" ]; then | ||
| release_notes="Use this branch to push the emergency fix. Please replace this PR description with a description of the fix after it has been pushed." | ||
|
|
||
| { | ||
| echo "release_notes<<$OUTPUT_DELIM" | ||
| echo "$release_notes" | ||
| echo "$OUTPUT_DELIM" | ||
| } >> "$GITHUB_OUTPUT" | ||
|
|
||
| { | ||
| echo "RELEASE_NOTES<<$OUTPUT_DELIM" | ||
| echo "$release_notes" | ||
| echo "$OUTPUT_DELIM" | ||
| } >> "$GITHUB_ENV" | ||
| exit 0 |
There was a problem hiding this comment.
The script uses 'exit 0' to terminate early for emergency releases, which is correct. However, the OUTPUT_DELIM variable is declared twice in the function - once at line 128 for both emergency and regular releases, and would be declared again at line 169 (removed in this PR). While the current implementation works, consider clarifying that OUTPUT_DELIM is used in both paths by moving its declaration to the top of the step or adding a comment explaining why it's declared before the conditional.
| - name: Create bump-version branch | ||
| run: | | ||
| git checkout main | ||
| git pull origin main | ||
| # Delete bump-version branch if it exists locally | ||
| git branch -D bump-version 2>/dev/null || true | ||
| # Delete bump-version branch if it exists remotely | ||
| git push origin --delete bump-version 2>/dev/null || true | ||
| git checkout -b bump-version |
There was a problem hiding this comment.
The bump-version branch is always created from main, even for emergency releases. This could lead to an inconsistent state where:
- The emergency release branch has version X based on an old tag
- The bump-version PR updates main to version X, but main has newer commits
This creates confusion about which version is which. Consider conditionally creating the bump-version PR only for regular releases from main, or adjusting the logic to handle version conflicts between emergency releases and main.
| } | ||
|
|
||
| - name: Create bump-version branch | ||
| run: | |
There was a problem hiding this comment.
When checking out main at line 259, if the previous workflow steps modified the working tree state, this checkout could fail or produce unexpected results. The workflow should ensure a clean state before switching branches.
Consider adding "git reset --hard" or starting from a clean checkout of main to ensure there are no lingering changes from the release branch work.
| run: | | |
| run: | | |
| # Ensure a clean working tree before switching branches | |
| git reset --hard |
| # Emergency releases branching from previous release skip initial QA builds | ||
| # as they should be manually triggered after the emergency fixes are pushed | ||
| - name: Trigger dev builds (initial QA) | ||
| if: env.BRANCH_FROM == 'main' |
There was a problem hiding this comment.
The workflow uses BRANCH_FROM environment variable for the conditional check, but in GitHub Actions, environment variables in 'if' conditions should be accessed using the expression syntax. The current syntax "env.BRANCH_FROM == 'main'" may not work as expected in all contexts.
Use the proper expression syntax: "${{ env.BRANCH_FROM == 'main' }}" or compare against the input directly: "${{ inputs.branch_from == 'main' }}"
| if: env.BRANCH_FROM == 'main' | |
| if: ${{ env.BRANCH_FROM == 'main' }} |
| run: | | ||
| set -eo pipefail | ||
|
|
||
| BRANCH_FROM="${{ env.BRANCH_FROM }}" |
There was a problem hiding this comment.
The validation action successfully normalizes the tag format (adding "v" prefix if missing) and outputs it, but the original env.BRANCH_FROM still contains the unnormalized value. This is used in line 127 for the release notes condition check, which could cause the emergency release detection to fail if the user inputs "1.9.24" instead of "v1.9.24".
The workflow should use the normalized value from steps.validate_branch_from.outputs.branch_or_tag consistently throughout, or update the BRANCH_FROM environment variable with the normalized value.
| BRANCH_FROM="${{ env.BRANCH_FROM }}" | |
| BRANCH_FROM="${{ steps.validate_branch_from.outputs.branch_or_tag }}" |
| # Check if base branch already exists remotely | ||
| if git ls-remote --heads origin "$BASE_BRANCH" | grep -q "$BASE_BRANCH"; then | ||
| echo "Base branch $BASE_BRANCH already exists, checking it out" | ||
| git checkout "$BASE_BRANCH" | ||
| git pull origin "$BASE_BRANCH" | ||
| else | ||
| echo "Creating base branch $BASE_BRANCH from tag $BRANCH_FROM" | ||
| git checkout -b "$BASE_BRANCH" | ||
| git push --set-upstream origin "$BASE_BRANCH" | ||
| fi |
There was a problem hiding this comment.
If the base branch is created from a tag (lines 106-108), it's immediately pushed to origin. However, if this step fails or the workflow is re-run, the base branch might already exist remotely but not match the tag exactly (if someone pushed commits to it).
The current logic checks if the base branch exists and pulls it (lines 101-104), but doesn't verify that it still points to the same commit as the tag. This could result in creating an emergency release from a modified base branch rather than the pristine tag state.
Consider adding verification that the base branch (if it exists) points to the same commit as the tag, or force-reset it to the tag.
| RELEASE_BRANCH="release/v${APP_VERSION}" | ||
| BASE_BRANCH="main" | ||
| else | ||
| RELEASE_BRANCH="emergency-release/v${APP_VERSION}" |
There was a problem hiding this comment.
The check_release_branch job in android.yml and ios.yml workflows looks for active 'release/v*' branches to skip scheduled builds. However, this PR introduces 'emergency-release/v*' branches which won't match this pattern.
This means scheduled nightly builds will continue to run even when an emergency release branch is active, which may not be the desired behavior. Consider updating the check in android.yml and ios.yml to also look for 'emergency-release/v*' branches, or document why emergency releases shouldn't block scheduled builds.
| RELEASE_BRANCH="emergency-release/v${APP_VERSION}" | |
| RELEASE_BRANCH="release/v${APP_VERSION}-emergency" |
What
This PR implements some improvements for the
New Releaseworkflow based on what we've discussed here, especially in regards to adding support foremergency releases.Included in this PR:
New Releaseworkflow now opens a separateBump version to v{version}PR so we can immediately bump the app version numbers on themainbranch (sample bump version PR)New Releaseworkflow now allow us tobranch from a previous release tag(instead ofmain) in order to shipemergency releaseswithout having to worry about latest changes merged to main (sample emergency release PR)workflow understands it's an emergency releaseand create the release branch asemergency-release/v{version}and open a PR with title[Emergency Release] v{version}so it's clear this is an emergency releaseIf an invalid tag version is used the workflow logs will indicate it as below:
Why
So devs have
less manual workto do when issuing either a regular release or emergency release.Known limitations
The Release Process Doc still needs to be updated with more comprehensive
instructionsin regards toregular releasesandemergency releases. This will be done soon this week.Checklist
PR structure
Testing
Release