Actually check Rietveld for LGTMs in CheckOwners()

presubmit_canned_checks.py

Index: presubmit_canned_checks.py
diff --git a/presubmit_canned_checks.py b/presubmit_canned_checks.py
index 29967f54d22b26873ad0ac8f65250f8c3452c25d..97a248f8f18e072798bf053088e92c7d0359ad4f 100644
--- a/presubmit_canned_checks.py
+++ b/presubmit_canned_checks.py
@@ -626,16 +626,20 @@ def CheckBuildbotPendingBuilds(input_api, output_api, url, max_pendings,
   return []
 
 
-def CheckOwners(input_api, output_api, source_file_filter=None):
+def CheckOwners(input_api, output_api, host_url, email_regexp,
+    source_file_filter=None):
   affected_files = set([f.LocalPath() for f in
       input_api.change.AffectedFiles(source_file_filter)])
   owners_db = input_api.owners_db
+  email_regexp = input_api.re.compile(email_regexp)
+  owners_db.email_regexp = email_regexp
 
   if input_api.is_committing:
+    approvers = _Approvers(input_api, host_url, email_regexp)
     missing_files = owners_db.files_not_covered_by(affected_files,
-                                                input_api.change.approvers)
+                                                   approvers)
     if missing_files:
-      return [output_api.PresubmitError('Missing owner LGTM for: %s' %
+      return [output_api.PresubmitError('Missing LGTM from an OWNER for: %s' %
               ','.join(missing_files))]
     return []
   elif input_api.change.tags.get('R'):
@@ -643,3 +647,29 @@ def CheckOwners(input_api, output_api, source_file_filter=None):
 
   suggested_reviewers = owners_db.reviewers_for(affected_files)
   return [output_api.PresubmitAddText('R=%s' % ','.join(suggested_reviewers))]
+
+
+def _Approvers(input_api, host_url, email_regexp):

[M-A Ruel, 2011/03/10 13:37:54]

I don't think we want this there. This means that *all* tbr review would now
require committing with --bypass-hooks / --no_presubmit ...

+  if not input_api.change.issue:
+    return []
+
+  path = '/api/%s?messages=true'
+  url = (host_url + path) % input_api.change.issue
+
+  f = input_api.urllib2.urlopen(url)
+  issue_props = input_api.json.load(f)
+  owner = input_api.re.escape(issue_props['owner'])
+
+  # TODO(dpranke): This mimics the logic in
+  # /tools/commit-queue/verifiers/reviewer_lgtm.py
+  # We should share the code and/or remove the check there where it is
+  # redundant (since the commit queue also enforces the presubmit checks).
+  def match_reviewer(r):
+    return email_regexp.match(r) and not input_api.re.match(owner, r)
+
+  approvers = []
+  for m in issue_props['messages']:
+    if 'lgtm' in m['text'].lower() and match_reviewer(m['sender']):
+      approvers.append(m['sender'])
+  return set(approvers)
+