Copilot commented on code in PR #50813:
URL: https://github.com/apache/arrow/pull/50813#discussion_r3719414864
##########
dev/release/03-binary-submit.sh:
##########
@@ -34,7 +34,15 @@ version_with_rc="${version}-rc${rc}"
crossbow_job_prefix="release-${version_with_rc}"
release_tag="apache-arrow-${version}-rc${rc}"
rc_branch="release-${version_with_rc}"
-maint_branch="maint-${version}"
+
+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
Review Comment:
Same as in 02-source.sh: unquoted `echo ${version} | cut ...` plus `-eq`
under `set -e` makes the script brittle and can yield a hard-to-understand
failure on unexpected version formats. Using parameter expansion (and
optionally validating X.Y.Z) avoids subprocesses and produces a clearer error
early.
##########
docs/source/developers/release.rst:
##########
@@ -164,12 +168,12 @@ Create or update the corresponding maintenance branch
.. code-block::
# Execute the following from an up to date main branch.
- # This will create a branch locally called maint-X.Y.Z.
+ # This will create the maintenance branch locally.
# 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>
Review Comment:
The docs now use a placeholder `<maintenance-branch>` but don’t explain how
to determine the actual branch name (maint-X.Y.Z vs maint-X.Y.x). This can
still lead to confusion when pushing the newly created branch (especially for
patch releases). Suggest either documenting the naming explicitly here or
pushing the current branch name automatically.
##########
dev/release/02-source.sh:
##########
@@ -40,7 +40,16 @@ rc=$2
. "${SOURCE_DIR}/utils-env.sh"
tag=apache-arrow-${version}-rc${rc}
-maint_branch=maint-${version}
+
+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
Review Comment:
The version parsing uses `echo ${version} | cut ...` without quoting and
then does an integer `-eq` test under `set -e`. If the version is malformed
(e.g. missing a patch component or includes a suffix), this will fail with a
confusing `integer expression expected`/exit rather than a clear error.
Consider validating the version format and using parameter expansion to avoid
subprocesses and word-splitting.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
