Jeremy White : Do not send MR changes on apparent maintainer.

[Alexandre Julliard]

Module: tools
Branch: master
Commit: 894a49d2cb0069a98b962e5136a5d86d17230340
URL:    https://source.winehq.org/git/tools.git/?a=commit;h=894a49d2cb0069a98b962e5136a5d86d17230340

Author: Jeremy White <jwhite at codeweavers.com>
Date:   Thu Apr 28 10:13:46 2022 -0500

Do not send MR changes on apparent maintainer.

If we think that it was a maintainer and they only changed meta
information, then do not send a new copy of the MR.

Signed-off-by: Alexandre Julliard <julliard at winehq.org>

---

 gitlab/gitlab-to-mail/gitlabtomail.py | 41 +++++++++++++++++++++++++++--------
 1 file changed, 32 insertions(+), 9 deletions(-)

diff --git a/gitlab/gitlab-to-mail/gitlabtomail.py b/gitlab/gitlab-to-mail/gitlabtomail.py
index 3708679..a2f7079 100755
--- a/gitlab/gitlab-to-mail/gitlabtomail.py
+++ b/gitlab/gitlab-to-mail/gitlabtomail.py
@@ -467,7 +467,14 @@ def fixup_version(mail, version):
     mail['Subject'] = re.sub(r'PATCH ([0-9])', f"PATCH v{version} \\1", subject)
 
 
-def commits_changed(new, old):
+def done_by_maintainer(author, commits):
+    for commit in commits:
+        if commit['committer_name'] != author:
+            return True
+    return False
+
+
+def commits_meta_changed(new, old):
     if len(old) != len(new):
         return True
     for i in range(0, len(old)):
@@ -477,7 +484,7 @@ def commits_changed(new, old):
     return False
 
 
-def get_changes(iid, version, versionid, old_versionid):
+def get_changes(iid, version, versionid, old_versionid, mr):
     current_changes = fetch_mr_version(iid, versionid)
     old_changes = fetch_mr_version(iid, old_versionid)
     old_commits = []
@@ -487,12 +494,25 @@ def get_changes(iid, version, versionid, old_versionid):
     version_string = f"  v{version}: "
     vstring_len = len(version_string)
 
+    # After a merge, the source branch is deleted, which will short
+    #  circuit this logic.  We cannot do any analysis on an MR in
+    #  that condition.
+    if len(old_changes['diffs']) == 0:
+        return None
+
     # We don't want to send an email notice on a rebase or other innocuous change
+    #   But this gets complicated.  There is a strong sense that if an
+    #   author makes a change to their commit messages or description,
+    #   that warrants another email.  But if a maintainer adds a signed off
+    #   or makes a similar small change at merge time, we *don't* want that.
+    # So we try to figure out if this is a maintainer merge, and if so
+    #   we want to more aggressively 'quiet' this logic.  So basically,
+    #   if the messages are not changed, or only changed by a maintainer, let's be quiet.
     if 'commits' in current_changes and 'commits' in old_changes:
-        # If the commit messages / composition has not changed and
-        if not commits_changed(current_changes['commits'], old_changes['commits']):
+        changed = commits_meta_changed(current_changes['commits'], old_changes['commits'])
+        maintainer = done_by_maintainer(mr['author']['name'], current_changes['commits'])
+        if maintainer or not changed:
             if 'diffs' in current_changes and 'diffs' in old_changes:
-                # The diff is unchanged, then...
                 if current_changes['diffs'] == old_changes['diffs']:
                     return None
 
@@ -510,7 +530,11 @@ def get_changes(iid, version, versionid, old_versionid):
     return "\n" + version_string + changes
 
 
-def create_cover(mr_id, mr_iid, mr_version, versions, nr_patches, author_name, title, description):
+def create_cover(mr_id, mr_iid, mr_version, versions, nr_patches, mr):
+    author_name = f"{mr['author']['name']} (@{mr['author']['username']})"
+    title = mr['title']
+    description = mr['description']
+
     mail = email.message.Message()
     mail['From'] = email.utils.formataddr((author_name, settings.BRIDGE_FROM_EMAIL))
     vstring = ""
@@ -523,7 +547,7 @@ def create_cover(mr_id, mr_iid, mr_version, versions, nr_patches, author_name, t
             #   versions array is ordered, which Arek seems to not have found
             vindex = mr_version * -1
             oindex = (mr_version - 1) * -1
-            changes = get_changes(mr_iid, mr_version, versions[vindex]['id'], versions[oindex]['id'])
+            changes = get_changes(mr_iid, mr_version, versions[vindex]['id'], versions[oindex]['id'], mr)
             if changes is None:
                 return None
 
@@ -583,8 +607,7 @@ def process_mr(mr, update_db):
         return
 
     nr_patches = len(patches)
-    author = f"{mr['author']['name']} (@{mr['author']['username']})"
-    cover = create_cover(mr['id'], iid, version, versions, nr_patches, author, mr['title'], mr['description'])
+    cover = create_cover(mr['id'], iid, version, versions, nr_patches, mr)
     if cover is None:
         log(f"MR{iid}v{version} - skipping, has no changes")
         return