git_cl: description fetching from code review servers.

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 distutils.version import LooseVersion
@@ skipping 2503 matching lines @@
     gerrit_util.SetReview(self._GetGerritHost(), self.GetIssue(),
                           labels={'Commit-Queue': vote_map[new_state]})
 
 
 _CODEREVIEW_IMPLEMENTATIONS = {
   'rietveld': _RietveldChangelistImpl,
   'gerrit': _GerritChangelistImpl,
 }
 
 
 def _add_codereview_select_options(parser):

[tandrii(chromium), 2016/04/18 22:13:16]

for consistency, please use this func and the one below it.

   """Appends --gerrit and --rietveld options to force specific codereview."""
   parser.codereview_group = optparse.OptionGroup(
       parser, 'EXPERIMENTAL! Codereview override options')
   parser.add_option_group(parser.codereview_group)
   parser.codereview_group.add_option(
       '--gerrit', action='store_true',
       help='Force the use of Gerrit for codereview')
   parser.codereview_group.add_option(
       '--rietveld', action='store_true',
       help='Force the use of Rietveld for codereview')
@@ skipping 714 matching lines @@
   if options.json_file:
     with open(options.json_file, 'wb') as f:
       json.dump(summary, f)
   return 0
 
 
 def CMDdescription(parser, args):
   """Brings up the editor for the current CL's description."""
   parser.add_option('-d', '--display', action='store_true',
                     help='Display the description instead of opening an editor')
+  parser.add_option('-i', '--issue', type=int,
+                    help='The issue to fetch the description from')

[tandrii(chromium), 2016/04/18 22:13:16]

s/from/from, instead of the current branch issue.

or maybe even better way to put it so default is obvious?
also, it might be easier for users to just copy-paste URL from browser, and
expect it to work, similar to git cl patch. The codereview server will them be
inferred from it.

[martiniss, 2016/04/19 00:18:53]

Changed. And made it so you can copy paste a URL.

+  parser.add_option('--gerrit', action='store_true', default=False,
+                    help='The issue is stored in gerrit')
+  parser.add_option('--rietveld', action='store_true', default=False,
+                    help='The issue is stored in rietveld')
+  parser.add_option('-s', '--server',

[tandrii(chromium), 2016/04/18 22:13:16]

do you actually use this field? If you do, then make use of it for gerrit as
well + document what this is - url or hostname, unless both are OK.

[martiniss, 2016/04/19 00:18:53]

Removed.

+                    help='The codereview server the issue is stored at')
+
   auth.add_auth_options(parser)
   options, _ = parser.parse_args(args)
+
+  if options.gerrit and options.rietveld:

[tandrii(chromium), 2016/04/18 22:13:16]

see _process_codereview_select_options

[martiniss, 2016/04/19 00:18:53]

Done.

+    DieWithError('only one of --gerrit and --rietveld is allowed')
+  if not (options.gerrit or options.rietveld) and (
+      options.issue or options.server):
+    DieWithError('one of --gerrit and --rietveld is required')

[tandrii(chromium), 2016/04/18 22:13:16]

why is this so? it wasn't required before. and the codereview server can be
guessed from local repo (settings, or remote url, or whatever).

[martiniss, 2016/04/19 00:18:53]

Removed.

+
+  codereview = None
+  if options.gerrit:
+    codereview = 'gerrit'
+  if options.rietveld:
+    codereview = 'rietveld'
+
   auth_config = auth.extract_auth_config_from_options(options)
-  cl = Changelist(auth_config=auth_config)
+  kwargs = {
+    'auth_config': auth_config,
+    'issue': options.issue,
+    'codereview': codereview,
+  }
+
+  if options.rietveld:
+    kwargs['rietveld_server'] = options.server
+  # else nothing, because gerrit guesses the code review from the git host, for

[tandrii(chromium), 2016/04/18 22:13:16]

so does rietveld, as it's the server is recorded in codereview.settings. 

[martiniss, 2016/04/19 00:18:53]

Removed.

+  # now
+  cl = Changelist(**kwargs)
   if not cl.GetIssue():
     DieWithError('This branch has no associated changelist.')
   description = ChangeDescription(cl.GetDescription())
   if options.display:
     print description.description
     return 0
   description.prompt()
   if cl.GetDescription() != description.description:
     cl.UpdateDescription(description.description)
   return 0
@@ skipping 1552 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)