Implement git cl set-commit --dry-run for Rietveld.

git_cl.py

 #!/usr/bin/env python
 # 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.
 
 # Copyright (C) 2008 Evan Martin <martine@danga.com>
 
 """A git-command for integrating reviews on Rietveld and Gerrit."""
 
 from __future__ import print_function
@@ skipping 1719 matching lines @@
     return 'waiting'
 
   def UpdateDescriptionRemote(self, description):
     return self.RpcServer().update_description(
         self.GetIssue(), self.description)
 
   def CloseIssue(self):
     return self.RpcServer().close_issue(self.GetIssue())
 
   def SetFlag(self, flag, value):
+    return self.SetFlags({flag: value})
+
+  def SetFlags(self, flags):
     """Patchset must match."""

[Michael Achenbach, 2016/07/06 07:47:59]

fly-by nit: This is not a method comment, rather a line comment. Maybe add a
useful method comment?

[tandrii(chromium), 2016/07/06 10:44:03]

Done.

     if not self.GetPatchset():
       DieWithError('The patchset needs to match. Send another patchset.')
     try:
-      return self.RpcServer().set_flag(
-          self.GetIssue(), self.GetPatchset(), flag, value)
+      return self.RpcServer().set_flags(
+          self.GetIssue(), self.GetPatchset(), flags)
     except urllib2.HTTPError as e:
       if e.code == 404:
         DieWithError('The issue %s doesn\'t exist.' % self.GetIssue())
       if e.code == 403:
         DieWithError(
             ('Access denied to issue %s. Maybe the patchset %s doesn\'t '
              'match?') % (self.GetIssue(), self.GetPatchset()))
       raise
 
   def RpcServer(self):
@@ skipping 25 matching lines @@
     return ['rietveldserver']
 
   def GetRieveldObjForPresubmit(self):
     return self.RpcServer()
 
   def SetCQState(self, new_state):
     props = self.GetIssueProperties()
     if props.get('private'):
       DieWithError('Cannot set-commit on private issue')
 
     if new_state == _CQState.COMMIT:

[Michael Achenbach, 2016/07/06 07:47:58]

The dry_run flag also changes the commit flag, but not vice-versa. Isn't that
somewhat inconsistent? I assume there's logic on the other side that takes care
of this. I'd find it more consistent if either dry-run only sets the dry-run
flag, of if commit and none also unset the dry-run flag.

Another suggestion: How about a tri-state? commit 0-1-2, where 2 is dry-run?
That'd render the need for a flags method unnecessary.

[tandrii(chromium), 2016/07/06 10:44:03]

well, this is rietveld API, and I'm not going to touch it.
In Gerrit, I actually use 0-1-2 semantic.

       self.SetFlag('commit', '1')
     elif new_state == _CQState.NONE:
       self.SetFlag('commit', '0')
     else:
-      raise NotImplementedError()
+      assert new_state == _CQState.DRY_RUN
+      self.SetFlags({'commit': '1', 'cq_dry_run': '1'})
 
 
   def CMDPatchWithParsedIssue(self, parsed_issue_arg, reject, nocommit,
                               directory):
     # TODO(maruel): Use apply_issue.py
 
     # PatchIssue should never be called with a dirty tree.  It is up to the
     # caller to check this, but just in case we assert here since the
     # consequences of the caller not checking this could be dire.
     assert(not git_common.is_dirty_git_tree('apply'))
@@ skipping 3290 matching lines @@
 if __name__ == '__main__':
   # These affect sys.stdout so do it outside of main() to simplify mocks in
   # unit testing.
   fix_encoding.fix_encoding()
   setup_color.init()
   try:
     sys.exit(main(sys.argv[1:]))
   except KeyboardInterrupt:
     sys.stderr.write('interrupted\n')
     sys.exit(1)