GH-50811: [Release] Use maint-Major.Minor.x for patch releases as the maintenance branch on required release scripts - #50813
GH-50811: [Release] Use maint-Major.Minor.x for patch releases as the maintenance branch on required release scripts#50813raulcd wants to merge 5 commits into
Conversation
…as the maintenance branch on required release scripts
|
|
There was a problem hiding this comment.
🟢 Ready to approve
The change directly addresses the reported release-branch selection bug with a small, low-risk update to the release scripts and aligned documentation.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Pull request overview
Fixes Arrow release tooling/docs so patch releases use the correct maintenance branch naming (maint-Major.Minor.x) instead of a version-specific branch (maint-X.Y.Z), preventing RC verification PR creation failures for patch releases (GH-50811).
Changes:
- Update
02-source.shto compute the maintenance branch asmaint-X.Y.xfor patch releases andmaint-X.Y.0for minor releases. - Update
03-binary-submit.shwith the same maintenance-branch selection logic for Crossbow PR reporting. - Clarify release documentation to state patch releases use
maint-X.Y.xand are based on the previous release tag.
File summaries
| File | Description |
|---|---|
| docs/source/developers/release.rst | Documents patch-release maintenance branch naming (maint-X.Y.x) and creation basis. |
| dev/release/02-source.sh | Uses maint-X.Y.x for patch releases when creating the RC verification PR. |
| dev/release/03-binary-submit.sh | Uses maint-X.Y.x for patch releases when reporting packaging jobs to the RC PR. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 3
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
| patch_version=$(echo ${version} | cut -d. -f3) | ||
| if [ "${patch_version}" -eq 0 ]; then | ||
| maint_branch="maint-${version}" | ||
| else | ||
| # Patch releases use the maint-Major.Minor.x branch. | ||
| major_minor_version=$(echo ${version} | cut -d. -f1-2) | ||
| maint_branch="maint-${major_minor_version}.x" | ||
| fi |
There was a problem hiding this comment.
I don't think this is necessary, wrong versions would have also failed previously.
| patch_version=$(echo ${version} | cut -d. -f3) | ||
| if [ "${patch_version}" -eq 0 ]; then | ||
| maint_branch="maint-${version}" | ||
| else | ||
| # Patch releases use the maint-Major.Minor.x branch. | ||
| major_minor_version=$(echo ${version} | cut -d. -f1-2) | ||
| maint_branch="maint-${major_minor_version}.x" | ||
| fi |
| # X.Y.Z corresponds with the Major, Minor and Patch version number | ||
| # of the release respectively. As an example 9.0.0 | ||
| archery release cherry-pick X.Y.Z --execute | ||
| # Push the maintenance branch to the remote repository | ||
| git push -u upstream maint-X.Y.Z | ||
| git push -u upstream <maintenance-branch> |
There was a problem hiding this comment.
I don't think that's necessary either.
tadeja
left a comment
There was a problem hiding this comment.
Uff, probably fine, yeah? (as I'm really not versed with all this, and even the beaten up 🤖 agrees there's nothing crucial to add)
Just some cosmetic doc suggestions below, but not required for this PR.
Co-authored-by: tadeja <tadeja@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
The documentation updates include a duplicated checklist step and broken RST structure/indentation in the patch-release merge dropdown that should be corrected before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
Suppressed comments (2)
docs/source/developers/release.rst:202
- The “test before creating a release candidate” checklist now has two consecutive bullets for creating a PR; the first still references
maint-X.Y.Z, which conflicts with the new<maintenance-branch>placeholder and reads as a duplicated step.
* Create a pull request from the up-to-date maint-X.Y.Z branch onto main
* Create a pull request from the up-to-date ``<maintenance-branch>`` onto main
* Title the pull request "WIP: Dummy PR to check <maintenance-branch> status"
docs/source/developers/release.rst:358
- The added “maint-X.Y.x” merge instructions are not indented under the dropdown, which will break the RST structure, and the earlier code-block lost its final
git pushcommand. Since this dropdown is for patch releases, themaint-X.Y.Zvariant should be removed to avoid conflicting guidance.
Merge ``release-X.Y.Z-rcN`` to ``maint-X.Y.x``:
.. code-block:: Bash
# git checkout maint-25.0.x
- Files reviewed: 3/3 changed files
- Comments generated: 0 new
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
There was a problem hiding this comment.
🟡 Changes recommended
The updated release documentation has a formatting/indentation issue that will render incorrectly and introduces inconsistent patch-release merge instructions.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
Suppressed comments (1)
docs/source/developers/release.rst:357
- This dropdown section’s indentation is broken (lines after the first code-block are no longer indented under the dropdown), which will render incorrectly in Sphinx. It also leaves the first example without an actual
git pushcommand and appears to contradict the earlier guidance that patch releases merge intomaint-X.Y.x(notmaint-X.Y.Z). Consider collapsing this dropdown to a single, correctly-indentedmaint-X.Y.xexample.
git push -u upstream maint-X.Y.x
.. dropdown:: Add the new release to the Apache Reporter System
:animate: fade-in-slide-down
:class-title: sd-fs-5
- Files reviewed: 3/3 changed files
- Comments generated: 0 new
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
There was a problem hiding this comment.
🟡 Changes recommended
The updated maintenance-branch logic doesn’t handle “minor release” naming (maint-<major>.x.x) as used by archery release cherry-pick, so minor releases could still fail by targeting a non-existent base branch.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 2
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
| patch_version=$(echo ${version} | cut -d. -f3) | ||
| if [ "${patch_version}" -eq 0 ]; then | ||
| maint_branch="maint-${version}" | ||
| else | ||
| # Patch releases use the maint-Major.Minor.x branch. | ||
| major_minor_version=$(echo ${version} | cut -d. -f1-2) | ||
| maint_branch="maint-${major_minor_version}.x" | ||
| fi |
| patch_version=$(echo ${version} | cut -d. -f3) | ||
| if [ "${patch_version}" -eq 0 ]; then | ||
| maint_branch="maint-${version}" | ||
| else | ||
| # Patch releases use the maint-Major.Minor.x branch. | ||
| major_minor_version=$(echo ${version} | cut -d. -f1-2) | ||
| maint_branch="maint-${major_minor_version}.x" | ||
| fi |
Rationale for this change
When working with patch releases currently the maintenance branch used is not correct and the scripts for 02-source and 03-binary fail to create the PR or find it.
What changes are included in this PR?
Use the correct maintenance branch format taking into account the differences between major releases and patch releases.
Updated release documentation to make it clearer that only the
maint-X.Y.xbranch is used for patch releases.Are these changes tested?
I've validated the modified shell script and the cut commands. I haven't used them directly as the RC PR and jobs are already created.
I've validated no other scripts use the maintenance branch.
Are there any user-facing changes?
No