Use m['approval'] instead of looking at the text for 'lgtm'.

presubmit_canned_checks.py

 # Copyright (c) 2010 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.
 
 """Generic presubmit checks that can be reused by other presubmit checks."""
 
 
 ### Description checks
 
 def CheckChangeHasTestField(input_api, output_api):
@@ skipping 743 matching lines @@
 
 
 def _RietveldOwnerAndApprovers(input_api, email_regexp):
   """Return the owner and approvers of a change, if any."""
   # TODO(dpranke): Should figure out if input_api.host_url is supposed to
   # be a host or a scheme+host and normalize it there.
   host = input_api.host_url
   if not host.startswith('http://') and not host.startswith('https://'):
     host = 'http://' + host
   url = '%s/api/%s?messages=true' % (host, input_api.change.issue)
+  issue_props = input_api.json.load(input_api.urllib2.urlopen(url))
+  owner_email = issue_props['owner_email']
 
-  f = input_api.urllib2.urlopen(url)
-  issue_props = input_api.json.load(f)
+  def match_reviewer(r):
+    return email_regexp.match(r) and r != owner_email
+
   messages = issue_props.get('messages', [])
-  owner_email = issue_props['owner_email']
-  owner_regexp = input_api.re.compile(input_api.re.escape(owner_email))
-  approvers = _GetApprovers(messages, email_regexp, owner_regexp)
+  approvers = set(
+      m['sender'] for m in messages
+      if m.get('approval') and match_reviewer(m['sender']))
 
-  return (owner_email, set(approvers))
-
-
-def _IsApprovingComment(text):
-  """Implements the logic for parsing a change comment for approval."""
-
-  # Any comment that contains a non-quoted line containing an 'lgtm' is an
-  # approval.
-  #
-  # TODO(dpranke): this differs from the logic used inside Google in a few
-  # ways. Inside Google,
-  #
-  # 1) the approving phrase must appear at the beginning of the first non
-  #    quoted-line in the comment.'
-  # 2) "LG", "Looks Good", and "Looks Good to Me" are also acceptable.
-  # 3) Subsequent comments from the reviewer can rescind approval, unless
-  #    the phrase "LGTM++" was used.
-  # We should consider implementing some or all of this here.
-  for l in text.splitlines():
-    l = l.strip().lower()
-    if l.startswith('>'):
-      continue
-
-    if 'lgtm' in l:
-      return True
-  return False
-
-
-def _GetApprovers(messages, email_regexp, owner_regexp):
-  """Returns the set of approvers for a change given the owner and messages.
-
-  Messages should be a list of dicts containing 'sender' and 'text' keys."""
-
-  # 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 owner_regexp.match(r)
-
-  approvers = []
-  for m in messages:
-    sender = m['sender']
-    if _IsApprovingComment(m['text']) and match_reviewer(sender):
-      approvers.append(sender)
-  return set(approvers)
+  return owner_email, approvers
 
 
 def _CheckConstNSObject(input_api, output_api, source_file_filter):
   """Checks to make sure no objective-c files have |const NSSomeClass*|."""
   pattern = input_api.re.compile(
       r'const\s+NS(?!(Point|Range|Rect|Size)\s*\*)\w*\s*\*')
 
   def objective_c_filter(f):
     return (source_file_filter(f) and
             input_api.os_path.splitext(f.LocalPath())[1] in ('.h', '.m', '.mm'))
@@ skipping 122 matching lines @@
     results.extend(input_api.canned_checks.CheckChangeSvnEolStyle(
         input_api, output_api, source_file_filter=text_files))
     snapshot("checking svn mime types")
     results.extend(input_api.canned_checks.CheckSvnForCommonMimeTypes(
         input_api, output_api))
     snapshot("checking license")
     results.extend(input_api.canned_checks.CheckLicense(
         input_api, output_api, license_header, source_file_filter=sources))
   snapshot("done")
   return results