-
Notifications
You must be signed in to change notification settings - Fork 538
Fix patch release workflow to use NEXT_RELEASE for version updates #19303
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
|
This pull request does not have a backport label. Could you fix it @Copilot? 🙏
|
Co-authored-by: isaacaflores2 <[email protected]>
…p-to-next-patch-<next-release>'
|
I tested the upcoming patch releases on my fork (details) in the description. You can see the PRs created here: https://github.com/isaacaflores2/apm-server/pulls. The PRs include multiple backup files (.bck). I am not sure why this is happening but I do not think it is release to any of my changes. @v1v please let me know if you disagree or know why the backup files are being created. @carsonip If there are no concerns with this PR and you would like to use the updated patch-release. Feel free to merge this PR and backport. Otherwise I can make any other changes, so the team can you this on the subsequent patch release(s). |
carsonip
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.
is the github workflow version box description still accurate with this change? Or can we make it even clearer?
I bet, copilot created them, I've seen that behaviour in the past |
| NEXT_PROJECT_MINOR_VERSION ?= $(PROJECT_MAJOR_VERSION).$(shell expr $(PROJECT_MINOR_VERSION) + 1).0 | ||
| NEXT_RELEASE ?= $(RELEASE_BRANCH).$(shell expr $(PROJECT_PATCH_VERSION) + 1) | ||
| BRANCH_PATCH = update-$(NEXT_RELEASE) | ||
| BRANCH_PATCH = bump-to-next-patch-$(NEXT_RELEASE) |
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'm not sure about this change, this branch is patch agnostic here. If a patch, then go to L57
IMO, let's keep the previous naming for now
I didn't merge this PR. Used the old workflow. It worked fine. TBH my only complaint about the process is just that the github workflow textbox description around |
Problem
The
patch-releasetarget inrelease.mkhad an inconsistency that caused confusion during patch releases. When releasing version8.18.7, the workflow would:update-8.18.8(usingNEXT_RELEASE)8.18.7(usingRELEASE_VERSION)This was confusing because the branch name suggested updating to
8.18.8, but the actual version updates pointed to8.18.7.Solution
Updated the
patch-releasetarget to consistently useNEXT_RELEASE(which is automatically calculated asRELEASE_VERSION + 1) throughout the workflow:update-versionto use$(NEXT_RELEASE)instead of$(RELEASE_VERSION)$(NEXT_RELEASE)$(NEXT_RELEASE)Result
Now when releasing version
8.18.7, the workflow correctly:update-8.18.88.18.8All references are now consistent! This eliminates the need for manual calculation and makes the workflow's intent clear: preparing the repository for the next version after the release.
Testing
Tested on forked repository against the branches
8.19and9.2export GITHUB_TOKEN=<token>release.mkchanges to each branchmake patch-releasefor each versionPatch 8.19
-> Created PR: isaacaflores2#1
Patch 9.1
-> Created PR: isaacaflores2#4
-> Created release notes PR: isaacaflores2#5
Patch 9.2
-> Created PR: isaacaflores2#2
patch-releaseso this PR includes changes to that file-> Created release notes PR: 9.2: update release notes isaacaflores2/apm-server#3
Original prompt
Implements #18853
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.