Add presubmit check to verify issue is not closed.

presubmit_canned_checks.py

 # Copyright (c) 2012 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."""
 
 import os as _os
 _HERE = _os.path.dirname(_os.path.abspath(__file__))
 
 
@@ skipping 773 matching lines @@
       suggested_owners = owners_db.reviewers_for(affected_files)
       output_list.append(output('Suggested OWNERS:\n    %s' %
                                 ('\n    '.join(suggested_owners))))
     return output_list
 
   if input_api.is_committing and not reviewers:
     return [output('Missing LGTM from someone other than %s' % owner_email)]
   return []
 
 
+def CheckIssueNotClosed(input_api, output_api):
+  """Verify issue is not closed.
+
+  We should not be working with a closed review. CQ and dcommit set this bit,
+  so it is a pretty good indicator of whether an issue has been committed.
+  """
+
+  issue = input_api.change.issue
+  if issue:
+    issue_props = input_api.rietveld.get_issue_properties(

[M-A Ruel, 2012/11/19 13:04:01]

input_api.rietveld could be None in the case it's gerrit CL IIRC, so on 802,
check if issue and input_api.rietveld:

[Isaac (away), 2012/11/21 20:24:32]

Done, added to _GetRietveldIssueProps in latest patchset.

+        issue=int(issue), messages=False) 

[M-A Ruel, 2012/11/19 13:04:01]

Is the int() call necessary? If so, it's a bug in the Change object IMHO.

[Isaac (away), 2012/11/21 20:24:32]

It was required, fixed in latest patchset.

+    if issue_props['closed']:
+      return [output_api.PresubmitError(
+          'Issue %s is closed.  If this issue was already used for a commit,\n'
+          'please reset the issue number associated with this branch with:\n'
+          'git cl issue 0\n' % issue
+          )]
+  return []
+

[M-A Ruel, 2012/11/19 13:04:01]

2 lines

[Isaac (away), 2012/11/21 20:24:32]

Done.

 def _RietveldOwnerAndReviewers(input_api, email_regexp, approval_needed=False):
   """Return the owner and reviewers of a change, if any.
 
   If approval_needed is True, only reviewers who have approved the change
   will be returned.
   """
   if not input_api.change.issue:
     return None, None
 
   issue_props = input_api.rietveld.get_issue_properties(
@@ skipping 129 matching lines @@
       delta_ms = int(1000*(dt2 - snapshot_memory[0]))
       if delta_ms > 500:
         print "  %s took a long time: %dms" % (snapshot_memory[1], delta_ms)
     snapshot_memory[:] = (dt2, msg)
 
   if owners_check:
     snapshot("checking owners")
     results.extend(input_api.canned_checks.CheckOwners(
         input_api, output_api, source_file_filter=None))
 
+  snapshot("checking review not closed")
+  results.extend(input_api.canned_checks.CheckIssueNotClosed(
+      input_api, output_api))
+
   snapshot("checking long lines")
   results.extend(input_api.canned_checks.CheckLongLines(
       input_api, output_api, source_file_filter=sources))
   snapshot( "checking tabs")
   results.extend(input_api.canned_checks.CheckChangeHasNoTabs(
       input_api, output_api, source_file_filter=sources))
   snapshot( "checking stray whitespace")
   results.extend(input_api.canned_checks.CheckChangeHasNoStrayWhitespace(
       input_api, output_api, source_file_filter=sources))
   snapshot("checking nsobjects")
@@ skipping 13 matching lines @@
     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("checking was uploaded")
     results.extend(input_api.canned_checks.CheckChangeWasUploaded(
         input_api, output_api))
   snapshot("done")
   return results