Merge pull request #377 from Mark-Simulacrum/optional-at

Adjust reviewer selection to work on unprefixed group selection

highfive/newpr.py

@@ -33,8 +33,7 @@
 review_with_reviewer = 'r? @%s\n\n(rust-highfive has picked a reviewer for you, use r? to override)'
 review_without_reviewer = '@%s: no appropriate reviewer found, use r? to override'
 
-reviewer_re = re.compile(r"\b[rR]\?[:\- ]*@([a-zA-Z0-9\-]+)")
-reviewer_group_re = re.compile(r"\b[rR]\?[:\- ]*@?(?:([a-zA-Z0-9\-]+)/)([a-zA-Z0-9\-]+)")
+reviewer_re = re.compile(r"\b[rR]\?[:\- ]*(?:@?([a-zA-Z0-9\-]+)/)?(@?[a-zA-Z0-9\-]+)")
 submodule_re = re.compile(r".*\+Subproject\scommit\s.*", re.DOTALL | re.MULTILINE)
 target_re = re.compile("^[+-]{3} [ab]/compiler/rustc_target/src/spec/", re.MULTILINE)
 
@@ -232,7 +231,7 @@ def is_new_contributor(self, username, owner, repo):
                 raise e
 
     def get_groups(self):
-        groups = deepcopy(self.repo_config['groups'])
+        groups = deepcopy(self.repo_config['groups'] if 'groups' in self.repo_config else {})
 
         # fill in the default groups, ensuring that overwriting is an
         # error.
@@ -249,16 +248,18 @@ def find_reviewer(self, msg, exclude):
         None.
         """
         if msg is not None:
-            match = reviewer_group_re.search(msg)
+            match = reviewer_re.search(msg)
             if match:
                 groups = self.get_groups()
                 potential = groups.get(match.group(2)) or groups.get("%s/%s" % (match.group(1), match.group(2))) or []
-                potential.extend(groups["all"])
-                return self.pick_reviewer(groups, potential, exclude)
-
-            match = reviewer_re.search(msg)
-            if match:
-                return match.group(1)
+                if 'all' in groups:
+                    potential.extend(groups["all"])
+                picked = self.pick_reviewer(groups, potential, exclude)
+                if picked:
+                    return picked
+                if match.group(1) is None and match.group(2):
+                    if match.group(2).startswith('@'):
+                        return match.group(2)[1:]
 
 
     def choose_reviewer(self, repo, owner, diff, exclude):

highfive/tests/fakes.py

@@ -86,7 +86,7 @@ def get_repo_configs():
             }
         },
         'teams': {
-            "groups": {"all": [], "a": ["@pnkfelix"], "b/c": ["@nrc"]}
+            "groups": {"all": [], "a": ["@pnkfelix"], "d": ["@e"], "compiler-team": ["@niko"], "b/c": ["@nrc"]}
         }
     }
 

highfive/tests/test_newpr.py

@@ -348,7 +348,8 @@ def test_find_reviewer(self):
             # https://github.com/rust-lang-nursery/highfive/issues/184
             None,
         )
-        handler = HighfiveHandlerMock(Payload({})).handler
+        config = {}
+        handler = HighfiveHandlerMock(Payload({}), repo_config=config).handler
 
         for (msg, reviewer) in found_cases:
             assert handler.find_reviewer(msg, None) == reviewer, \
@@ -1280,8 +1281,15 @@ def test_with_team_ping(self):
         found_cases = (
             ("r? @foo/a", "pnkfelix"),
             ("r? foo/a", "pnkfelix"),
+            ("r? rust-lang/compiler-team", "niko"),
+            ("r? compiler-team", "niko"),
             ("r? @b/c", "nrc"),
             ("r? b/c", "nrc"),
+
+            # @d goes to the user
+            ("r? @d", "d"),
+            # d goes to the team
+            ("r? d", "e"),
         )
 
         not_found_cases = (