-
Notifications
You must be signed in to change notification settings - Fork 265
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
Support creating release branches when releasing new versions #1627
Support creating release branches when releasing new versions #1627
Conversation
@@ -57,7 +57,9 @@ jobs: | |||
- name: "Store version numbers in env variables" | |||
run: | | |||
echo RELEASE_VERSION=${{ inputs.version }} >> $GITHUB_ENV | |||
echo RELEASE_VERSION_WITHOUT_STABILITY=$(echo ${{ inputs.version }} | awk -F- '{print $1}') >> $GITHUB_ENV |
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.
This is necessary as we need to check for a patch release. The best way is to cut the suffix (e.g. -RC0
) from the version and only keep the major, minor, and patch numbers.
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.
Noted that this is using the -F sepstring
syntax from awk(1p). It wasn't immediately clear to me due to the omitted space, but https://askubuntu.com/a/342850 helped clarify.
@@ -90,7 +111,7 @@ jobs: | |||
EOL | |||
|
|||
- name: "Create draft release" | |||
run: echo "RELEASE_URL=$(gh release create ${{ inputs.version }} --target ${{ github.ref_name }} --title "${{ inputs.version }}" --notes-file release-message --draft)" >> "$GITHUB_ENV" | |||
run: echo "RELEASE_URL=$(gh release create ${{ inputs.version }} --target ${{ env.RELEASE_BRANCH }} --title "${{ inputs.version }}" --notes-file release-message --draft)" >> "$GITHUB_ENV" |
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.
Using the (potentially newly created) release branch ensures that GitHub shows the correct "x commits to " when looking at a release on GitHub.
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.
Could you please add some tests? Joking aside, it's not easy to review without tests to make sure there aren't any special cases left out. The amount of logic embedded in Yaml is borderline, so if it grows, we'll have to think of a separate script.
.github/workflows/release.yml
Outdated
- name: "Create new release branch" | ||
if: ${{ github.ref_name != env.RELEASE_BRANCH }} | ||
run: | | ||
git checkout -b ${{ env.RELEASE_BRANCH }} | ||
git push origin ${{ env.RELEASE_BRANCH }} |
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.
For reproducibility, if the branch already exists (because the job has already been launched but failed when calling the GitHub API, for example), then the git push
will fail.
We can accept this and consider deleting the branch manually in this case. Or you can check whether the branch exists and is up-to-date.
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.
Good point.
What we can do is that we check out all branches (similar to the merge-up workflow) and switch to that branch in certain instances. Those can be releasing a non-patch release from the development branch when the release branch already exists instead of creating the new branch. The only issue with that is that instead of using the release workflow from that branch, it runs the release workflow from whatever branch was used. This shouldn't be a big issue in most cases, but it's important to keep in mind.
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.
Turns out, we're already using this kind of logic in the Laravel integration: https://github.com/mongodb/laravel-mongodb/blob/e373350f436596402a2b51cd7a8fe68fdd2df848/.github/workflows/release.yml#L44-L67
With that in mind, I'd update the logic here to be the same as in the Laravel integration, and improve on that in a separate step. We can also consider extracting the logic to drivers-github-tools, which already holds some driver-specific actions.
@GromNaN agree with the amount of logic in the workflows. If it makes you feel any better, the Java driver has been using very similar logic for a while now, apparently I just forgot to also add this to the PHP driver when I created it for Java. |
@@ -57,7 +57,9 @@ jobs: | |||
- name: "Store version numbers in env variables" | |||
run: | | |||
echo RELEASE_VERSION=${{ inputs.version }} >> $GITHUB_ENV | |||
echo RELEASE_VERSION_WITHOUT_STABILITY=$(echo ${{ inputs.version }} | awk -F- '{print $1}') >> $GITHUB_ENV |
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.
Noted that this is using the -F sepstring
syntax from awk(1p). It wasn't immediately clear to me due to the omitted space, but https://askubuntu.com/a/342850 helped clarify.
Co-authored-by: Jeremy Mikola <[email protected]>
This PR adds branching logic to the release workflow to support creating new non-patch releases from development branches without having to manually push the release branch.
In summary:
A.B.0
), including pre-releases can be created from either thevA.B
release branch or thevA.x
development branch.A.B.C
), including pre-releases have to be created from thevA.B
release branch