Show a list of directories missing OWNER reviewers on upload, and show directories rather than file…

owners.py

Index: owners.py
===================================================================
--- owners.py	(revision 125361)
+++ owners.py	(working copy)
@@ -76,28 +76,36 @@
     return self._covering_set_of_owners_for(files)
 
   def files_are_covered_by(self, files, reviewers):

[Dirk Pranke, 2012/03/08 07:19:48]

I meant that I don't think anyone calls this routine at all, so you can just
delete it.

-    """Returns whether every file is owned by at least one reviewer."""
-    return not self.files_not_covered_by(files, reviewers)
+    """Returns whether every file is owned by at least one reviewer.
 
-  def files_not_covered_by(self, files, reviewers):
-    """Returns the set of files that are not owned by at least one reviewer.
+    If reviewers is empty, returns False if there are any files at all.
+    """
+    if files and not reviewers:
+      return False
+    return not self.directories_not_covered_by(files, reviewers)
 
+  def directories_not_covered_by(self, files, reviewers):
+    """Returns the set of directories that are not owned by a reviewer.
+
+    Determines which of the given files are not owned by at least one of the
+    reviewers, then returns a set containing the applicable enclosing
+    directories, i.e. the ones upward from the files that have OWNERS files.
+
     Args:
         files is a sequence of paths relative to (and under) self.root.
-        reviewers is a sequence of strings matching self.email_regexp."""
+        reviewers is a sequence of strings matching self.email_regexp.
+    """
     self._check_paths(files)
     self._check_reviewers(reviewers)
-    if not reviewers:
-      return files
 
     self._load_data_needed_for(files)
     files_by_dir = self._files_by_dir(files)

[Dirk Pranke, 2012/03/08 07:19:48]

You no longer need the files, just the dirs, so you can do:

dirs = self._files_by_dir(files).keys()
covered_dirs = self._dirs_covered_by(reviewers)
uncovered_dirs = filter(lambda d: not self._is_dir_covered_by(d, covered_dirs,
dirs)
return set(self._enclosing_dir_with_owners(d) for d in uncovered_dirs)

(or something like it).

[Pam (message me for reviews), 2012/03/08 09:11:54]

Thanks, I should have seen that. No more coding at midnight for me.

I took out _files_by_dir(), too, since it's also no longer needed.

     covered_dirs = self._dirs_covered_by(reviewers)
-    uncovered_files = []
+    uncovered_dirs = []
     for d, files_in_d in files_by_dir.iteritems():
       if not self._is_dir_covered_by(d, covered_dirs):
-        uncovered_files.extend(files_in_d)
-    return set(uncovered_files)
+        uncovered_dirs.append(self._enclosing_dir_with_owners(d))
+    return set(uncovered_dirs)
 
   def _check_paths(self, files):
     def _is_under(f, pfx):
@@ -129,6 +137,15 @@
       dirname = self.os_path.dirname(dirname)
     return dirname in covered_dirs
 
+  def _enclosing_dir_with_owners(self, directory):
+    """Returns the innermost enclosing directory that has an OWNERS file."""
+    dirpath = directory
+    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):
     for f in files:
       dirpath = self.os_path.dirname(f)