Add presubmit check for dcommitted / CQed CLs

presubmit_canned_checks.py

Index: presubmit_canned_checks.py
diff --git a/presubmit_canned_checks.py b/presubmit_canned_checks.py
index af88a39b1e8b35099655b3ac1688edfb33c6171e..bbd409297f4e286297267fc599a9b75f3e1774f2 100644
--- a/presubmit_canned_checks.py
+++ b/presubmit_canned_checks.py
@@ -5,6 +5,8 @@
 """Generic presubmit checks that can be reused by other presubmit checks."""
 
 import os as _os
+import re

[M-A Ruel, 2012/11/22 15:47:06]

why not input_api.re?

[Isaac (away), 2012/11/22 18:50:52]

I can do that -- is there a reason we're avoiding the python re class here?

[Dirk Pranke, 2012/11/22 18:54:07]

input_api.re would just be a pointer to the python module. You'll notice we
don't normally import anything directly into this file (I'm actually surprised
that "import os" has crept in). This makes it easier to mock out objects for
testing, and I seem to recall there's an actual reason to limit imports as well
(although that reason might've been obsolete).

[Isaac (away), 2012/11/25 02:12:44]

OK, fixed.

+
 _HERE = _os.path.dirname(_os.path.abspath(__file__))
 
 
@@ -46,9 +48,9 @@ def CheckChangeHasQaField(input_api, output_api):
 
 
 def CheckDoNotSubmitInDescription(input_api, output_api):
-  """Checks that the user didn't add 'DO NOT ' + 'SUBMIT' to the CL description.
+  """Checks that the user didn't add 'DO NOT SUBMIT' to the CL description.

[M-A Ruel, 2012/11/22 15:47:06]

Why? This would block the CQ from processing this file.

[Isaac (away), 2012/11/22 18:50:52]

I didn't realize that was the reason for splitting this up.  OK.  We can use
pythons string concatenation and change to 'DO NOT ''SUBMIT', which is preferred
python style.

[Isaac (away), 2012/11/25 02:12:44]

Fixed.

   """
-  keyword = 'DO NOT ' + 'SUBMIT'
+  keyword = 'DO NOT SUBMIT'
   if keyword in input_api.change.DescriptionText():
     return [output_api.PresubmitError(
         keyword + ' is present in the changelist description.')]
@@ -67,6 +69,17 @@ def CheckChangeHasDescription(input_api, output_api):
   return []
 
 
+def CheckChangeDescriptionNotCommitted(input_api, output_api):
+  """Checks that the CL does not end with a Committed link."""
+  description = input_api.change.DescriptionText()
+  if re.match('\nCommitted: \S+$', description):

[M-A Ruel, 2012/11/22 15:47:06]

You could use ^ instead with re.search(.., re.MULTILINE)

Also, regexp string should be of r type.

[Isaac (away), 2012/11/22 18:50:52]

I only want to match Committed on the last line, so multiline wouldn't work.  A
revert which quotes the original commit might have a "Committed" line from the
original commit, but not as the last line.

+    return [output_api.PresubmitError(
+      'The CL description appears to end with a committed link. If this issue\n'
+      'has been previously used for a commit, please make a new issue number\n'
+      'by typing "git cl issue 0" and reuploading your CL.')]
+  return []
+
+
 def CheckChangeWasUploaded(input_api, output_api):
   """Checks that the issue was uploaded before committing."""
   if input_api.is_committing and not input_api.change.issue:
@@ -78,10 +91,10 @@ def CheckChangeWasUploaded(input_api, output_api):
 ### Content checks
 
 def CheckDoNotSubmitInFiles(input_api, output_api):
-  """Checks that the user didn't add 'DO NOT ' + 'SUBMIT' to any files."""
+  """Checks that the user didn't add 'DO NOT SUBMIT' to any files."""
   # We want to check every text file, not just source files.
   file_filter = lambda x : x
-  keyword = 'DO NOT ' + 'SUBMIT'
+  keyword = 'DO NOT SUBMIT'
   errors = _FindNewViolationsOfRule(lambda _, line : keyword not in line,
                                     input_api, file_filter)
   text = '\n'.join('Found %s in %s' % (keyword, loc) for loc in errors)
@@ -971,5 +984,15 @@ def PanProjectChecks(input_api, output_api,
     snapshot("checking was uploaded")
     results.extend(input_api.canned_checks.CheckChangeWasUploaded(
         input_api, output_api))
+    snapshot("checking description")
+    results.extend(input_api.canned_checks.CheckChangeHasDescription(
+        input_api, output_api))
+    results.extend(input_api.canned_checks.CheckDoNotSubmitInDescription(
+        input_api, output_api))
+    results.extend(input_api.canned_checks.CheckChangeDescriptionNotCommitted(
+        input_api, output_api))
+    snapshot("checking do not submit in files")
+    results.extend(input_api.canned_checks.CheckDoNotSubmitInFiles(
+        input_api, output_api))
   snapshot("done")
   return results