Merge pull request #336 from JohnTitor/ping-without-dirs

Ping reviewers even if `dirs` key is empty

Author: pietroalbini

highfive/newpr.py

@@ -315,39 +315,39 @@ def get_to_mention(self, diff, author):
         """
         Get the list of people to mention.
         """
-        dirs = self.repo_config.get('dirs', {})
         mentions = self.repo_config.get('mentions', {})
+        if not mentions:
+            return []
 
         to_mention = set()
         # If there's directories with specially assigned groups/users
         # inspect the diff to find the directory with the most additions
-        if dirs:
-            cur_dir = None
-            for line in diff.split('\n'):
-                if line.startswith("diff --git "):
-                    # update cur_dir
-                    cur_dir = None
-                    parts = line[line.find(" b/") + len(" b/"):].split("/")
-                    if not parts:
-                        continue
-                    cur_dir = "/".join(parts[:2])
-                    full_dir = "/".join(parts)
+        cur_dir = None
+        for line in diff.split('\n'):
+            if line.startswith("diff --git "):
+                # update cur_dir
+                cur_dir = None
+                parts = line[line.find(" b/") + len(" b/"):].split("/")
+                if not parts:
+                    continue
+                cur_dir = "/".join(parts[:2])
+                full_dir = "/".join(parts)
 
-                    # A few heuristics to get better reviewers
-                    if cur_dir.startswith('src/librustc'):
-                        cur_dir = 'src/librustc'
-                    if cur_dir == 'src/test':
-                        cur_dir = None
-                    if len(full_dir) > 0:
-                        for entry in mentions:
-                            # Check if this entry is a prefix
-                            eparts = entry.split("/")
-                            if (len(eparts) <= len(parts) and
-                                all(a==b for a,b in zip(parts, eparts))
-                            ):
-                                to_mention.add(entry)
-                            elif entry.endswith('.rs') and full_dir.endswith(entry):
-                                to_mention.add(entry)
+                # A few heuristics to get better reviewers
+                if cur_dir.startswith('src/librustc'):
+                    cur_dir = 'src/librustc'
+                if cur_dir == 'src/test':
+                    cur_dir = None
+                if len(full_dir) > 0:
+                    for entry in mentions:
+                        # Check if this entry is a prefix
+                        eparts = entry.split("/")
+                        if (len(eparts) <= len(parts) and
+                            all(a==b for a,b in zip(parts, eparts))
+                        ):
+                            to_mention.add(entry)
+                        elif entry.endswith('.rs') and full_dir.endswith(entry):
+                            to_mention.add(entry)
 
         mention_list = []
         for mention in to_mention:

highfive/tests/fakes.py

@@ -65,6 +65,16 @@ def get_repo_configs():
                     "reviewers": ["@pnkfelix"],
                 }
             }
+        },
+        'mentions_without_dirs': {
+            "groups": {"all": ["@JohnTitor"]},
+            "dirs": {},
+            "mentions": {
+                "README.md": {
+                    "message": "should be pinged",
+                    "reviewers": ["@JohnTitor"],
+                }
+            }
         }
     }
 

highfive/tests/fakes/mentions-without-dirs.diff

@@ -0,0 +1,12 @@
+diff --git a/README.md b/README.md
+index 5ec94e189f8..b35fbba3fcd 100644
+--- a/README.md
++++ b/README.md
+@@ -1,6 +1,6 @@
+ # The Rust Programming Language
+ 
+-This is the main source code repository for [Rust]. It contains the compiler,
++Hey! This is the main source code repository for [Rust]. It contains the compiler,
+ standard library, and documentation.
+ 
+ [Rust]: https://www.rust-lang.org

highfive/tests/test_newpr.py

@@ -1033,6 +1033,7 @@ def make_fakes(cls):
                 'normal': load_fake('normal.diff'),
                 'travis-yml': load_fake('travis-yml.diff'),
                 'mentions': load_fake('mentions.diff'),
+                'mentions-without-dirs': load_fake('mentions-without-dirs.diff'),
             },
             'config': fakes.get_repo_configs(),
             'global_': fakes.get_global_configs(),
@@ -1206,6 +1207,15 @@ def test_mentions(self):
         # @ehuss should not be listed here
         assert set(["@pnkfelix", "@GuillaumeGomez"]) == mentions
 
+    def test_mentions_without_dirs(self):
+        """Test pinging people even if the dirs key is empty."""
+        self.handler = HighfiveHandlerMock(
+            Payload({}), repo_config=self.fakes['config']['mentions_without_dirs']
+        ).handler
+        (chosen_reviewers, mentions) = self.choose_reviewers(
+            self.fakes['diff']['mentions-without-dirs'], "@JohnTitor",
+        )
+        assert set(["@JohnTitor"]) == mentions
 
 class TestRun(TestNewPR):
     @pytest.fixture(autouse=True)