-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Build Workflow: Revert version bump if build job fails #33239
Build Workflow: Revert version bump if build job fails #33239
Conversation
02a4994
to
91fb525
Compare
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.
I think this looks like an OK addition.
One scenario that occurred to me (which may never actually happen). If a PR is merged after the version bump occurs, and either one fails, could it result in an incorrectly deleted branch?
|
||
- name: Delete release branch if it was only just created for the RC | ||
if: | | ||
github.event.inputs.version == 'rc' && |
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.
Would this ever cause problems after the first RC? For example, a failure is encountered when bumping to RC2, 3, 4 etc.
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.
It's been a while since I've worked on this, but doesn't the criterion on the following line rule that out? I.e. verify that the previous version wasn't an RC.
I'm not totally sure I'm reading you correctly, but the deletion step is referring to the release branch by its name, so I think we should be safe here? |
dc1e476
to
943dd24
Compare
Rebased. |
This reverts commit 76ff8e8.
Co-authored-by: Bernie Reiter <[email protected]>
943dd24
to
22e58c3
Compare
Gave this another round of testing on my personal fork. I'm reasonably confident that this is working as it should be, so I'll go ahead and merge it 🙂 |
Description
In our plugin build workflow, which, when manually triggered, also contains a version bump step, and a step to create a release draft with the plugin build attached as an asset, the build step sometimes fails (e.g. because of npm flakiness) (see e.g.).
It's natural for people to try and re-trigger the step. However, this is also bound to fail, since at that point, the release branch has already been created, and the plugin version has already been bumped (see e.g.).
To prevent this, we can add another job to the workflow that upon build failure, reverts the version bump commit, and, if necessary, also deletes the release branch.
See this thread for more background, and potential alternative approaches: https://github.com/WordPress/gutenberg/pull/33105/files#r664591278
How has this been tested?
You'd basically need to fork the repo, force-push this branch to the fork's
trunk
, and add a commit that makes the build job reliably fail. Something likeand
For your convenience, I've done that on my fork 😬
Here's a test run for a
stable
release: https://github.com/ockham/gutenberg/actions/runs/1015794749.The
release/11.0
branch contains both the version bump commit (11.0.1
), and the revert: https://github.com/ockham/gutenberg/commits/release/11.0(Same for
trunk
: https://github.com/ockham/gutenberg/commits/trunk -- the commits are below the ones for the RC test.)Here's a test run for an
rc
release: https://github.com/ockham/gutenberg/actions/runs/1015804736The
release/11.1
branch was deleted after the build step failed; the version bump commit and revert for11.1.0-rc.1
are however present ontrunk
: https://github.com/ockham/gutenberg/commits/trunk