Issue 3674 - Add a hg hook that references commits in issues

sitescripts/hg/bin/update_issues.py

 #!/usr/bin/env python
 
 # This file is part of Adblock Plus <https://adblockplus.org/>,
 # Copyright (C) 2006-2016 Eyeo GmbH
 #
 # Adblock Plus is free software: you can redistribute it and/or modify
 # it under the terms of the GNU General Public License version 3 as
 # published by the Free Software Foundation.
 #
 # Adblock Plus is distributed in the hope that it will be useful,
@@ skipping 12 matching lines @@
 """
 
 import posixpath
 import re
 import xmlrpclib
 
 from sitescripts.utils import get_config, get_template
 
 def _generate_comments(repository_name, changes_by_issue):
   comments = {}
-  template = get_template("hg/template/issue_commit_comment.tmpl")

[Wladimir Palant, 2016/02/23 10:41:31]

I think you need autoescape=False here.

[Felix Dahlke, 2016/02/23 16:22:59]

Done.

+  template = get_template("hg/template/issue_commit_comment.tmpl",
+                          autoescape=False)
   for issue_id, changes in changes_by_issue.iteritems():
     comments[issue_id] = template.render({"repository_name": repository_name,
                                           "changes": changes})
   return comments
 
 def _post_comment(issue_id, comment):
   issue_id = int(issue_id)
   url = get_config().get("hg", "trac_xmlrpc_url")
   server = xmlrpclib.ServerProxy(url)
   attributes = server.ticket.get(issue_id)[3]
   server.ticket.update(issue_id, comment,
                        {"_ts": attributes["_ts"], "action": "leave"}, True)
 
 def hook(ui, repo, node, **kwargs):
   first_change = repo[node]
   issue_number_regex = re.compile(r"\bissue\s+(\d+)\b", re.I)
-  noissue_regex = re.compile(r"noissue\b", re.I)

[Wladimir Palant, 2016/02/23 10:41:31]

This should be \bnoissue\b (make sure that the word actually starts with
"noissue").

[Felix Dahlke, 2016/02/23 16:22:59]

I'm using match() so the message has to start with "noissue". We shouldn't match
"noissue" somewhere in the message here IMO.

[Wladimir Palant, 2016/02/23 21:33:23]

We agreed a while ago that we shouldn't be using match() to avoid exactly that
kind of non-obvious behavior. Please use search() and change the regular
expression into ^noissue\b.

[Felix Dahlke, 2016/02/23 21:40:58]

Fair enough. Tried to be smart, but this comment thread proves that it's a bad
idea. Done.

[Wladimir Palant, 2016/02/24 10:13:49]

Yes, particularly with the regexp and re.match() not in the same place this is
very confusing.

+  noissue_regex = re.compile(r"^noissue\b", re.I)
   changes_by_issue = {}
   for revision in xrange(first_change.rev(), len(repo)):
     change = repo[revision]
     description = change.description()
     issue_ids = issue_number_regex.findall(description)
     if issue_ids:
       for issue_id in issue_ids:
         changes_by_issue.setdefault(issue_id, []).append(change)
-    elif not noissue_regex.match(description):
+    elif not noissue_regex.search(description):
       # We should just reject all changes when one of them has an invalid
       # commit message format, see: https://issues.adblockplus.org/ticket/3679

[Wladimir Palant, 2016/02/23 10:41:31]

Is that a TODO comment? Maybe it should actually say TODO then.

[Felix Dahlke, 2016/02/23 16:22:59]

Well it's referencing an issue, I normally do just that without writing "TODO".

       ui.warn("warning: invalid commit message format in changeset %s\n" %
               change)
 
   repository_name = posixpath.split(repo.url())[1]
   comments = _generate_comments(repository_name, changes_by_issue)
 
   issue_url_template = get_config().get("hg", "issue_url_template")
   for issue_id, comment in comments.iteritems():
     try:
       _post_comment(issue_id, comment)
       ui.status("updating %s\n" % issue_url_template.format(id=issue_id))
     except:
       ui.warn("warning: failed to update %s\n" %
               issue_url_template.format(id=issue_id))