From 9b64c4b50192cc0218e8743e2d08ad9424c8fab8 Mon Sep 17 00:00:00 2001
From: Ahmon Dancy <adancy@wikimedia.org>
Date: Thu, 19 Jan 2023 10:10:16 -0800
Subject: [PATCH] Added some comments

---
 lib.py                    |  4 ++--
 patchforreview_remover.py | 25 +++++++++++++++++++++++--
 2 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/lib.py b/lib.py
index 7b37174..a9ecb6b 100755
--- a/lib.py
+++ b/lib.py
@@ -11,7 +11,7 @@ class Client(object):
 
     def __init__(self, url, username, key):
         self.url = url
-        self.username = username
+        self.username = username  # Unused
         self.column_cache = {}
         self.phid_cache = {}
         self.session = {
@@ -251,4 +251,4 @@ class Client(object):
             }
         }
         return self.post('maniphest.search', params)[
-            'data']
\ No newline at end of file
+            'data']
diff --git a/patchforreview_remover.py b/patchforreview_remover.py
index 9bcf0ba..7d7bb62 100755
--- a/patchforreview_remover.py
+++ b/patchforreview_remover.py
@@ -12,17 +12,29 @@ class Checker():
         self.client = client
 
     def check(self, t_id):
+        """
+        Returns true if the Patch-For-Review project should be removed from the Phabricator
+        task identified 't_id'.
+        """
         phid = self.client.lookupPhid(t_id)
         return self.phid_check(phid)
 
-    def phid_check(self, phid):
+    def phid_check(self, phid) -> bool:
+        """
+        Returns true if the Patch-For-Review project should be removed from the Phabricator
+        task identified by 'phid'.
+        """
         gerrit_bot_actions = []
+
+        # Note that transactions are returned in reverse chronological order (most recent first).
         for transaction in self.client.getTransactions(phid):
             if re.findall(re.escape('https://github.com/') + r'.+?/pull', str(transaction)):
                 return False
             if transaction['authorPHID'] == self.gerrit_bot_phid:
                 gerrit_bot_actions.append(transaction)
             else:
+                # If someone other than GerritBot adds the Patch-For-Review project, don't
+                # auto-remove it.
                 if transaction['type'] == 'projects':
                     check = self.project_patch_for_review_phid in str(
                         transaction['fields'])
@@ -44,9 +56,17 @@ class Checker():
                 r'Change \d+ \*\*(?:merged|abandoned)\*\* by ',
                 raw_comment)
 
+            # Append True or False depending on whether the action was to
+            # open a patch (True) or merge a patch (False)
             gerrit_patch_status[gerrit_patch_id].append(not(bool(merged)))
 
         for patch in gerrit_patch_status:
+            # The normal sequence of GerritBot transactions for a Gerrit change is "Change
+            # \d+ had a related patch set uploaded" (indicated by True in
+            # gerrit_patch_status) eventually followed by "Change \d+ (merged|abandoned)
+            # by whoever" (indicated by False in gerrit_patch_status).  The transactions
+            # are returned in reverse order so the opened/merged pattern will appear as
+            # the reverse of [True, False], which is [False, True].
             if gerrit_patch_status[patch] != [False, True]:
                 return False
         return True
@@ -54,9 +74,10 @@ class Checker():
 
 client = Client.newFromCreds()
 
+gerrit_bot_phid = 'PHID-USER-idceizaw6elwiwm5xshb'
 project_patch_for_review_phid = 'PHID-PROJ-onnxucoedheq3jevknyr'
 checker = Checker(
-    'PHID-USER-idceizaw6elwiwm5xshb',
+    gerrit_bot_phid,
     project_patch_for_review_phid,
     client)
 gen = client.getTasksWithProject(project_patch_for_review_phid)