From 91161b73bf10a07ef2751c6953a1eda20d372de8 Mon Sep 17 00:00:00 2001 From: Michael Rothgang Date: Fri, 2 May 2025 10:30:17 +0200 Subject: [PATCH 1/5] refactor: centralise check for 'does this message have any reaction with emojir X' --- scripts/zulip_emoji_merge_delegate.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/scripts/zulip_emoji_merge_delegate.py b/scripts/zulip_emoji_merge_delegate.py index aa500e87b09bd0..61a95fb6452a58 100644 --- a/scripts/zulip_emoji_merge_delegate.py +++ b/scripts/zulip_emoji_merge_delegate.py @@ -68,12 +68,15 @@ content = message['content'] # Check for emoji reactions reactions = message['reactions'] - has_peace_sign = any(reaction['emoji_name'] == 'peace_sign' for reaction in reactions) - has_bors = any(reaction['emoji_name'] == 'bors' for reaction in reactions) - has_merge = any(reaction['emoji_name'] == 'merge' for reaction in reactions) - has_awaiting_author = any(reaction['emoji_name'] == 'writing' for reaction in reactions) - has_maintainer_merge = any(reaction['emoji_name'] == 'hammer' for reaction in reactions) - has_closed = any(reaction['emoji_name'] == 'closed-pr' for reaction in reactions) + # Does this message have any reaction with an emoji |name|? + has_reaction = lambda name: any(reaction['emoji_name'] == name for reaction in reactions) + + has_peace_sign = has_reaction('peace_sign') + has_bors = has_reaction('bors') + has_merge = has_reaction('merge') + has_awaiting_author = has_reaction('writing') + has_maintainer_merge = has_reaction('hammer') + has_closed = has_reaction('closed-pr') first_in_thread = hashPR.search(message['subject']) and message['display_recipient'] == 'PR reviews' and message['subject'] not in first_by_subject first_by_subject[message['subject']] = message['id'] match = urlPR.search(content) or first_in_thread From 81e5faf50c8b86ed72df61d2cc2de14e92ba6c57 Mon Sep 17 00:00:00 2001 From: Michael Rothgang Date: Fri, 2 May 2025 10:33:57 +0200 Subject: [PATCH 2/5] refactor: deduplicate removing reactions This fixes the message for delegate emojis, to say 'delegated' and not 'peace_sign'. --- scripts/zulip_emoji_merge_delegate.py | 30 ++++++++------------------- 1 file changed, 9 insertions(+), 21 deletions(-) diff --git a/scripts/zulip_emoji_merge_delegate.py b/scripts/zulip_emoji_merge_delegate.py index 61a95fb6452a58..9c8957e32215ec 100644 --- a/scripts/zulip_emoji_merge_delegate.py +++ b/scripts/zulip_emoji_merge_delegate.py @@ -86,13 +86,16 @@ # removing previous emoji reactions # if the emoji is a custom emoji, add the fields `emoji_code` and `reaction_type` as well print("Removing previous reactions, if present.") - if has_peace_sign: - print('Removing peace_sign') + def remove_reaction(name: str, emoji_name: str) -> none: + print(f'Removing {name}') result = client.remove_reaction({ "message_id": message['id'], - "emoji_name": "peace_sign" + "emoji_name": emoji_name }) print(f"result: '{result}'") + + if has_peace_sign: + remove_reaction('delegated', 'peace_sign') if has_bors: print('Removing bors') result = client.remove_reaction({ @@ -103,26 +106,11 @@ }) print(f"result: '{result}'") if has_merge: - print('Removing merge') - result = client.remove_reaction({ - "message_id": message['id'], - "emoji_name": "merge" - }) - print(f"result: '{result}'") + remove_reaction('merge', 'merge') if has_maintainer_merge: - print('Removing maintainer-merge') - result = client.remove_reaction({ - "message_id": message['id'], - "emoji_name": "hammer" - }) - print(f"result: '{result}'") + remove_reaction('maintainer-merge', 'hammer') if has_awaiting_author: - print('Removing awaiting-author') - result = client.remove_reaction({ - "message_id": message['id'], - "emoji_name": "writing" - }) - print(f"result: '{result}'") + remove_reaction('awaiting-author', 'writing') if has_closed: print('Removing closed-pr') result = client.remove_reaction({ From 0d360304306448216d54dea83386e2ed36bbd459 Mon Sep 17 00:00:00 2001 From: Michael Rothgang Date: Fri, 2 May 2025 10:37:22 +0200 Subject: [PATCH 3/5] chore: deduplicate adding emoji reactions also --- scripts/zulip_emoji_merge_delegate.py | 40 ++++++++------------------- 1 file changed, 11 insertions(+), 29 deletions(-) diff --git a/scripts/zulip_emoji_merge_delegate.py b/scripts/zulip_emoji_merge_delegate.py index 9c8957e32215ec..9daa22f71c7064 100644 --- a/scripts/zulip_emoji_merge_delegate.py +++ b/scripts/zulip_emoji_merge_delegate.py @@ -124,42 +124,24 @@ def remove_reaction(name: str, emoji_name: str) -> none: # applying appropriate emoji reaction print("Applying reactions, as appropriate.") - if 'ready-to-merge' == LABEL: - print('adding ready-to-merge') - client.add_reaction({ + def add_reaction(name: str, emoji_name: str) -> none: + print(f'adding {name}') + result = client.add_reaction({ "message_id": message['id'], - "emoji_name": "bors" + "emoji_name": emoji_name }) + if 'ready-to-merge' == LABEL: + add_reaction('ready-to-merge', 'bors') elif 'delegated' == LABEL: - print('adding delegated') - client.add_reaction({ - "message_id": message['id'], - "emoji_name": "peace_sign" - }) + add_reaction('delegated', 'peace_sign') elif 'maintainer-merge' == LABEL: - print('adding maintainer-merge') - client.add_reaction({ - "message_id": message['id'], - "emoji_name": "hammer" - }) + add_reaction('maintainer-merge', 'hammer') elif LABEL == 'labeled': - print('adding awaiting-author') - client.add_reaction({ - "message_id": message['id'], - "emoji_name": "writing" - }) + add_reaction('awaiting-author', 'writing') elif LABEL == 'closed': - print('adding closed-pr') - client.add_reaction({ - "message_id": message['id'], - "emoji_name": "closed-pr" - }) + add_reaction('closed-pr', 'closed-pr') elif LABEL == 'unlabeled': print('awaiting-author removed') # the reaction was already removed. elif LABEL.startswith("[Merged by Bors]"): - print('adding [Merged by Bors]') - client.add_reaction({ - "message_id": message['id'], - "emoji_name": "merge" - }) + add_reaction('[Merged by Bors]', 'merge') From c4b24b7b195bbf63704460079a617c576c76ffcf Mon Sep 17 00:00:00 2001 From: Michael Rothgang Date: Fri, 2 May 2025 14:04:24 +0200 Subject: [PATCH 4/5] chore: also handle extra arguments in removal --- scripts/zulip_emoji_merge_delegate.py | 25 ++++++------------------- 1 file changed, 6 insertions(+), 19 deletions(-) diff --git a/scripts/zulip_emoji_merge_delegate.py b/scripts/zulip_emoji_merge_delegate.py index 9daa22f71c7064..9ef088f96cee1a 100644 --- a/scripts/zulip_emoji_merge_delegate.py +++ b/scripts/zulip_emoji_merge_delegate.py @@ -86,25 +86,19 @@ # removing previous emoji reactions # if the emoji is a custom emoji, add the fields `emoji_code` and `reaction_type` as well print("Removing previous reactions, if present.") - def remove_reaction(name: str, emoji_name: str) -> none: + def remove_reaction(name: str, emoji_name: str, **kwargs) -> none: print(f'Removing {name}') result = client.remove_reaction({ "message_id": message['id'], - "emoji_name": emoji_name + "emoji_name": emoji_name, + **kwargs }) print(f"result: '{result}'") if has_peace_sign: remove_reaction('delegated', 'peace_sign') if has_bors: - print('Removing bors') - result = client.remove_reaction({ - "message_id": message['id'], - "emoji_name": "bors", - "emoji_code": "22134", - "reaction_type": "realm_emoji", - }) - print(f"result: '{result}'") + remove_reaction("bors", "bors", emoji_code="22134", reaction_type="realm_emoji") if has_merge: remove_reaction('merge', 'merge') if has_maintainer_merge: @@ -112,15 +106,8 @@ def remove_reaction(name: str, emoji_name: str) -> none: if has_awaiting_author: remove_reaction('awaiting-author', 'writing') if has_closed: - print('Removing closed-pr') - result = client.remove_reaction({ - "message_id": message['id'], - "emoji_name": "closed-pr", - "emoji_code": "61293", # 61282 was the earlier version of the emoji - "reaction_type": "realm_emoji", - }) - print(f"result: '{result}'") - + # 61282 was the earlier version of the emoji + remove_reaction('closed-pr', 'closed-pr', emoji_code="61293", reaction_type="realm_emoji") # applying appropriate emoji reaction print("Applying reactions, as appropriate.") From f17197c039f6a38c9588c18c5042f89dec26b432 Mon Sep 17 00:00:00 2001 From: Michael Rothgang Date: Fri, 2 May 2025 14:04:36 +0200 Subject: [PATCH 5/5] Tweak --- scripts/zulip_emoji_merge_delegate.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/zulip_emoji_merge_delegate.py b/scripts/zulip_emoji_merge_delegate.py index 9ef088f96cee1a..af477a3d8492f9 100644 --- a/scripts/zulip_emoji_merge_delegate.py +++ b/scripts/zulip_emoji_merge_delegate.py @@ -84,9 +84,9 @@ print(f"matched: '{message}'") # removing previous emoji reactions - # if the emoji is a custom emoji, add the fields `emoji_code` and `reaction_type` as well + # If the emoji is a custom emoji, add the fields `emoji_code` and `reaction_type` as well. print("Removing previous reactions, if present.") - def remove_reaction(name: str, emoji_name: str, **kwargs) -> none: + def remove_reaction(name: str, emoji_name: str, **kwargs) -> None: print(f'Removing {name}') result = client.remove_reaction({ "message_id": message['id'],