Don't send mail for policy_checklist comments.

handlers/policy_checklist/handler.py

Index: handlers/policy_checklist/handler.py
diff --git a/handlers/policy_checklist/handler.py b/handlers/policy_checklist/handler.py
index ec26e37749cf3f5abf4070fbd979c720e320f989..3144cdf4492052b6ac1991e7a6fc9c35539073c9 100644
--- a/handlers/policy_checklist/handler.py
+++ b/handlers/policy_checklist/handler.py
@@ -31,6 +31,17 @@ JINJA_ENVIRONMENT = jinja2.Environment(
     loader=jinja2.FileSystemLoader(os.path.dirname(__file__)))
 
 
+def prepare_address_list(addr, email_list):
+  """Prepares |email_list| for use as rietveld query parameter.
+
+  Canonicalizes the entries in |email_list|, removes any occurrences of |addr|,
+  joins the entries with commas and returns the result.
+  """
+  return ','.join([item for canonicalized in map(util.get_emails, email_list)
+                   for item in canonicalized

[agable, 2013/08/27 14:49:19]

Double-nested fors in list comprehensions are ugly and imho unreadable. I'd
suggest turning this into two comprehensions for the sake of readability.

[Mattias Nissler (ping if slow), 2013/08/27 15:08:41]

Note that the nesting also flattens the list of lists, which I don't see how to
accomplish in two separate comprehensions. I've now switched to an
itertools.chain-based solution, I hope that is clearer.

[agable, 2013/08/27 15:56:26]

Ah, I'd missed the nesting from get_emails. Seems the rietveld is guaranteed to
return a list of strings, where each string is one reviewer. It would make more
sense to me to do a single list comprehension with "util.clean_email" which is
just a slightly different passthrough to email.utils.getaddresses (that asserts
it only gets back a single address, for example).

[Mattias Nissler (ping if slow), 2013/08/27 16:07:23]

Done.

+                   if item != addr])
+
+
 def process(addr, message, review, rietveld):
   """Handles reviews for chrome/app/policy/policy_templates.json.
 
@@ -78,4 +89,8 @@ def process(addr, message, review, rietveld):
   # Finally, post all inline comments.
   if len(chunks) > 0:
     template = JINJA_ENVIRONMENT.get_template(REVIEW_MESSAGE_TEMPLATE)
-    rietveld.post_comment(review.issue_id, template.render(review=review), True)
+    rietveld.publish_inline_comments(
+        review.issue_id, template.render(review=review),
+        prepare_address_list(addr, review.issue_data.reviewers),
+        prepare_address_list(addr, review.issue_data.cc),
+        send_mail=False)

[agable, 2013/08/27 14:49:19]

send_mail defaults to False.

[Mattias Nissler (ping if slow), 2013/08/27 15:08:41]

Done.