Copilot commented on code in PR #3818:
URL: https://github.com/apache/celeborn/pull/3818#discussion_r3822802435
##########
dev/merge_pr.py:
##########
@@ -242,6 +242,49 @@ def cherry_pick(pr_num, merge_hash, default_branch):
return pick_ref
+def _semver_max_version(names):
+ if not names:
+ return None
+ parsed = [(tuple(int(p) for p in n.split(".")), n) for n in names]
Review Comment:
`_semver_max_version` assumes every entry is a dot-separated numeric version
and will raise `ValueError` if it ever receives malformed input. Since this
helper is generic and could be reused, it would be more robust to either (a)
validate/skip non-conforming strings inside the function, or (b) assert the
expected format with a clear error message so failures are easier to diagnose.
##########
dev/merge_pr.py:
##########
@@ -242,6 +242,49 @@ def cherry_pick(pr_num, merge_hash, default_branch):
return pick_ref
+def _semver_max_version(names):
+ if not names:
+ return None
+ parsed = [(tuple(int(p) for p in n.split(".")), n) for n in names]
+ return max(parsed)[1]
+
+
+def compute_default_fix_versions(merge_branches, unreleased_version_names):
+ """
+ Compute the suggested fix versions for the merge branches.
+
+ main contributes the greatest unreleased N.0.0, and branch-M.N contributes
+ the greatest unreleased M.N.x version. Duplicate suggestions are collapsed.
+ """
+ default_fix_versions = []
+ for b in merge_branches:
+ if b == "main":
+ chosen = _semver_max_version(
+ [n for n in unreleased_version_names if
re.fullmatch(r"\d+\.0\.0", n)]
+ )
+ if chosen:
+ default_fix_versions.append(chosen)
+ else:
+ print(
+ "No unreleased N.0.0 fix version found on JIRA for main; "
+ "enter fix version(s) manually when prompted."
+ )
+ else:
+ prefix = b.replace("branch-", "")
+ chosen = _semver_max_version(
+ [n for n in unreleased_version_names if n.startswith(prefix)]
Review Comment:
`startswith(prefix)` can select the wrong minor series (e.g., prefix `0.7`
would also match `0.70.1`). This can produce an incorrect default fix version.
Prefer matching the branch prefix with a dot boundary (e.g., `^{prefix}\.\d+$`
with `re.fullmatch`, using `re.escape(prefix)`), so only `M.N.x` is considered
for `branch-M.N`.
--
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]
