An interactive tool to help find owners covering current change list.

owners.py

 # Copyright (c) 2012 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 """A database of OWNERS files.
 
 OWNERS files indicate who is allowed to approve changes in a specific directory
 (or who is allowed to make changes without needing approval of another OWNER).
 Note that all changes must still be reviewed by someone familiar with the code,
 so you may need approval from both an OWNER and a reviewer in many cases.
@@ skipping 93 matching lines @@
 
     # Pick a default email regexp to use; callers can override as desired.
     self.email_regexp = re.compile(BASIC_EMAIL_REGEXP)
 
     # Mapping of owners to the paths they own.
     self.owned_by = {EVERYONE: set()}
 
     # Mapping of paths to authorized owners.
     self.owners_for = {}
 
+    # Mapping reviewers to the most recent comment in the OWNERS files.
+    self.comments = {}
+
     # Set of paths that stop us from looking above them for owners.
     # (This is implicitly true for the root directory).
     self.stop_looking = set([''])
 
   def reviewers_for(self, files, author):
     """Returns a suggested set of reviewers that will cover the files.
 
     files is a sequence of paths relative to (and under) self.root.
     If author is nonempty, we ensure it is not included in the set returned
     in order avoid suggesting the author as a reviewer for their own changes."""
     self._check_paths(files)
-    self._load_data_needed_for(files)
+    self.load_data_needed_for(files)
     suggested_owners = self._covering_set_of_owners_for(files, author)
     if EVERYONE in suggested_owners:
       if len(suggested_owners) > 1:
         suggested_owners.remove(EVERYONE)
       else:
         suggested_owners = set(['<anyone>'])
     return suggested_owners
 
   def files_not_covered_by(self, files, reviewers):
     """Returns the files not owned by one of the reviewers.
 
     Args:
         files is a sequence of paths relative to (and under) self.root.
         reviewers is a sequence of strings matching self.email_regexp.
     """
     self._check_paths(files)
     self._check_reviewers(reviewers)
-    self._load_data_needed_for(files)
+    self.load_data_needed_for(files)
 
     covered_objs = self._objs_covered_by(reviewers)
     uncovered_files = [f for f in files
                        if not self._is_obj_covered_by(f, covered_objs)]
 
     return set(uncovered_files)
 
   def _check_paths(self, files):
     def _is_under(f, pfx):
       return self.os_path.abspath(self.os_path.join(pfx, f)).startswith(pfx)
@@ skipping 21 matching lines @@
 
   def _enclosing_dir_with_owners(self, objname):
     """Returns the innermost enclosing directory that has an OWNERS file."""
     dirpath = objname
     while not dirpath in self.owners_for:
       if self._stop_looking(dirpath):
         break
       dirpath = self.os_path.dirname(dirpath)
     return dirpath
 
-  def _load_data_needed_for(self, files):
+  def load_data_needed_for(self, files):
     for f in files:
       dirpath = self.os_path.dirname(f)
       while not dirpath in self.owners_for:
         self._read_owners_in_dir(dirpath)
         if self._stop_looking(dirpath):
           break
         dirpath = self.os_path.dirname(dirpath)
 
   def _read_owners_in_dir(self, dirpath):
     owners_path = self.os_path.join(self.root, dirpath, 'OWNERS')
     if not self.os_path.exists(owners_path):
       return
-
+    comment = ""

[M-A Ruel, 2013/04/24 13:04:45]

'' everywhere

[Bei Zhang, 2013/04/24 16:42:01]

Done.

+    last_line_is_comment = False

[M-A Ruel, 2013/04/24 13:04:45]

This is not needed.

comments = []
Then accumulate the comment lines with:
comments.append(line[1:].strip())
at lines 215 and 235, add
comments = []

that's all

[Bei Zhang, 2013/04/24 16:42:01]

That case I will have to introduce a last_comment variable because I cannot
clear it right away. It is referenced at line 233 in probably another iteration.

On 2013/04/24 13:04:45, Marc-Antoine Ruel wrote:

     lineno = 0
     for line in self.fopen(owners_path):
       lineno += 1
       line = line.strip()
-      if line.startswith('#') or line == '':
+      if line.startswith('#'):
+        if last_line_is_comment:
+          comment += " " + line[1:].strip()

[M-A Ruel, 2013/04/24 13:04:45]

why not keep the LF as-is?

[Bei Zhang, 2013/04/24 16:42:01]

I think the comment could be wrapped. Do you think I should keep them as is?

On 2013/04/24 13:04:45, Marc-Antoine Ruel wrote:

+        else:
+          comment = line[1:].strip()
+        last_line_is_comment = True
         continue
+      if line == '':
+        continue
+      last_line_is_comment = False
+
       if line == 'set noparent':
         self.stop_looking.add(dirpath)
         continue
 
       m = re.match("per-file (.+)=(.+)", line)
       if m:
         glob_string = m.group(1).strip()
         directive = m.group(2).strip()
         full_glob_string = self.os_path.join(self.root, dirpath, glob_string)
         if '/' in glob_string or '\\' in glob_string:
           raise SyntaxErrorInOwnersFile(owners_path, lineno,
               'per-file globs cannot span directories or use escapes: "%s"' %
               line)
         baselines = self.glob(full_glob_string)
         for baseline in (self.os_path.relpath(b, self.root) for b in baselines):
           self._add_entry(baseline, directive, "per-file line",
-                          owners_path, lineno)
+                          owners_path, lineno, comment)
         continue
 
       if line.startswith('set '):
         raise SyntaxErrorInOwnersFile(owners_path, lineno,
             'unknown option: "%s"' % line[4:].strip())
 
-      self._add_entry(dirpath, line, "line", owners_path, lineno)
+      self._add_entry(dirpath, line, "line", owners_path, lineno, comment)
 
-  def _add_entry(self, path, directive, line_type, owners_path, lineno):
+  def _add_entry(self, path, directive,
+                 line_type, owners_path, lineno, comment):
     if directive == "set noparent":
       self.stop_looking.add(path)
     elif self.email_regexp.match(directive) or directive == EVERYONE:
+      if directive not in self.comments:
+        self.comments[directive] = {}
+      self.comments[directive][path] = comment
       self.owned_by.setdefault(directive, set()).add(path)
       self.owners_for.setdefault(path, set()).add(directive)
     else:
       raise SyntaxErrorInOwnersFile(owners_path, lineno,
           ('%s is not a "set" directive, "*", '
            'or an email address: "%s"' % (line_type, directive)))
 
   def _covering_set_of_owners_for(self, files, author):
     dirs_remaining = set(self._enclosing_dir_with_owners(f) for f in files)
     all_possible_owners = self._all_possible_owners(dirs_remaining, author)
@@ skipping 46 matching lines @@
         if num_directories_owned:
           total_costs_by_owner[owner] = (total_distance /
                                          pow(num_directories_owned, 1.75))
 
     # Return the lowest cost owner. In the case of a tie, pick one randomly.
     lowest_cost = min(total_costs_by_owner.itervalues())
     lowest_cost_owners = filter(
       lambda owner: total_costs_by_owner[owner] == lowest_cost,
       total_costs_by_owner)
     return random.Random().choice(lowest_cost_owners)