From 033bb6055d28e156f618fb74640d1394b0015457 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Tue, 13 Feb 2024 12:09:13 +0200 Subject: [PATCH 1/2] Remove multiple commit prefixes Multiple commit prefixes "[X.Y] " were added in multilevel backports. Only the leftmost one matters. Others just increase the length of the title. --- cherry_picker/cherry_picker.py | 42 ++++++++++++++--------------- cherry_picker/test_cherry_picker.py | 27 +++++++++++++++++++ 2 files changed, 48 insertions(+), 21 deletions(-) diff --git a/cherry_picker/cherry_picker.py b/cherry_picker/cherry_picker.py index 715557f..f2046ee 100755 --- a/cherry_picker/cherry_picker.py +++ b/cherry_picker/cherry_picker.py @@ -330,12 +330,11 @@ def get_updated_commit_message(self, cherry_pick_branch): """ # Get the original commit message and prefix it with the branch name # if that's enabled. - commit_prefix = "" + updated_commit_message = self.get_commit_message(self.commit_sha1) if self.prefix_commit: - commit_prefix = f"[{get_base_branch(cherry_pick_branch)}] " - updated_commit_message = ( - f"{commit_prefix}{self.get_commit_message(self.commit_sha1)}" - ) + updated_commit_message = remove_commit_prefix(updated_commit_message) + base_branch = get_base_branch(cherry_pick_branch) + updated_commit_message = f"[{base_branch}] {updated_commit_message}" # Add '(cherry picked from commit ...)' to the message # and add new Co-authored-by trailer if necessary. @@ -443,6 +442,7 @@ def create_gh_pr(self, base_branch, head_branch, *, commit_message, gh_auth): request_headers = sansio.create_headers(self.username, oauth_token=gh_auth) title, body = normalize_commit_message(commit_message) if not self.prefix_commit: + title = remove_commit_prefix(title) title = f"[{base_branch}] {title}" data = { "title": title, @@ -880,19 +880,10 @@ def version_from_branch(branch): """ return version information from a git branch name """ - try: - return tuple( - map( - int, - re.match(r"^.*(?P\d+(\.\d+)+).*$", branch) - .groupdict()["version"] - .split("."), - ) - ) - except AttributeError as attr_err: - raise ValueError( - f"Branch {branch} seems to not have a version in its name." - ) from attr_err + m = re.search(r"\d+(?:\.\d+)+", branch) + if not m: + raise ValueError(f"Branch {branch} seems to not have a version in its name.") + return tuple(map(int, m[0].split("."))) def get_current_branch(): @@ -929,12 +920,21 @@ def normalize_commit_message(commit_message): """ Return a tuple of title and body from the commit message """ - split_commit_message = commit_message.split("\n") - title = split_commit_message[0] - body = "\n".join(split_commit_message[1:]) + title, _, body = commit_message.partition("\n") return title, body.lstrip("\n") +def remove_commit_prefix(commit_message): + """ + Remove prefix "[X.Y] " from the commit message + """ + while True: + m = re.match(r"\[\d+(?:\.\d+)+\] *", commit_message) + if not m: + return commit_message + commit_message = commit_message[m.end() :] + + def is_git_repo(): """Check whether the current folder is a Git repo.""" cmd = "git", "rev-parse", "--git-dir" diff --git a/cherry_picker/test_cherry_picker.py b/cherry_picker/test_cherry_picker.py index 0876440..809081a 100644 --- a/cherry_picker/test_cherry_picker.py +++ b/cherry_picker/test_cherry_picker.py @@ -30,6 +30,7 @@ load_config, load_val_from_git_cfg, normalize_commit_message, + remove_commit_prefix, reset_state, reset_stored_config_ref, set_state, @@ -558,6 +559,22 @@ def test_normalize_short_commit_message(): ) +def test_remove_commit_prefix(): + assert ( + remove_commit_prefix("[3.12] Fix something (GH-3113)") + == "Fix something (GH-3113)" + ) + assert ( + remove_commit_prefix("[3.11] [3.12] Fix something (GH-3113)") + == "Fix something (GH-3113)" + ) + assert remove_commit_prefix("Fix something (GH-3113)") == "Fix something (GH-3113)" + assert ( + remove_commit_prefix("[WIP] Fix something (GH-3113)") + == "[WIP] Fix something (GH-3113)" + ) + + @pytest.mark.parametrize( "commit_message,expected_commit_message", ( @@ -626,6 +643,16 @@ def test_normalize_short_commit_message(): Co-authored-by: PR Author Co-authored-by: PR Co-Author """, ), + # ensure the existing commit prefix is replaced + ( + "[3.7] [3.8] Fix broken `Show Source` links on documentation " + "pages (GH-3113) (GH-3114) (GH-3115)", + """[3.6] Fix broken `Show Source` links on documentation """ + """pages (GH-3113) (GH-3114) (GH-3115) +(cherry picked from commit b9ff498793611d1c6a9b99df464812931a1e2d69) + +Co-authored-by: PR Author """, + ), ), ) def test_get_updated_commit_message_with_trailers( From 24a0efae8ca8f630aa83a79e591869b29817ebd0 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Tue, 13 Feb 2024 14:57:03 +0200 Subject: [PATCH 2/2] Update cherry_picker/test_cherry_picker.py Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com> --- cherry_picker/test_cherry_picker.py | 25 +++++++++++-------------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/cherry_picker/test_cherry_picker.py b/cherry_picker/test_cherry_picker.py index 809081a..5138c83 100644 --- a/cherry_picker/test_cherry_picker.py +++ b/cherry_picker/test_cherry_picker.py @@ -559,20 +559,17 @@ def test_normalize_short_commit_message(): ) -def test_remove_commit_prefix(): - assert ( - remove_commit_prefix("[3.12] Fix something (GH-3113)") - == "Fix something (GH-3113)" - ) - assert ( - remove_commit_prefix("[3.11] [3.12] Fix something (GH-3113)") - == "Fix something (GH-3113)" - ) - assert remove_commit_prefix("Fix something (GH-3113)") == "Fix something (GH-3113)" - assert ( - remove_commit_prefix("[WIP] Fix something (GH-3113)") - == "[WIP] Fix something (GH-3113)" - ) +@pytest.mark.parametrize( + "commit_message, expected", + [ + ("[3.12] Fix something (GH-3113)", "Fix something (GH-3113)"), + ("[3.11] [3.12] Fix something (GH-3113)", "Fix something (GH-3113)"), + ("Fix something (GH-3113)", "Fix something (GH-3113)"), + ("[WIP] Fix something (GH-3113)", "[WIP] Fix something (GH-3113)"), + ], +) +def test_remove_commit_prefix(commit_message, expected): + assert remove_commit_prefix(commit_message) == expected @pytest.mark.parametrize(