git-cl: Remove _add_codereview_[issue_]select_options and _CODEREVIEW_IMPLEMENTATIONS.

Change-Id: I94922e1abab8acac132e16d255e1a9b95771ea5c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2040572Reviewed-by: Anthony Polito <apolito@google.com>
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>

git_cl.py

@@ -2777,31 +2777,6 @@ class Changelist(object):
     return [r['email'] for r in details['reviewers'].get('REVIEWER', [])]
 
 
-_CODEREVIEW_IMPLEMENTATIONS = {
-  'gerrit': Changelist,
-}
-
-
-def _add_codereview_issue_select_options(parser, extra=""):
-  _add_codereview_select_options(parser)
-
-  text = ('Operate on this issue number instead of the current branch\'s '
-          'implicit issue.')
-  if extra:
-    text += ' '+extra
-  parser.add_option('-i', '--issue', type=int, help=text)
-
-
-def _add_codereview_select_options(parser):
-  """Appends --gerrit option to force specific codereview."""
-  parser.codereview_group = optparse.OptionGroup(
-      parser, 'DEPRECATED! Codereview override options')
-  parser.add_option_group(parser.codereview_group)
-  parser.codereview_group.add_option(
-      '--gerrit', action='store_true',
-      help='Deprecated. Noop. Do not use.')
-
-
 def _get_bug_line_values(default_project, bugs):
   """Given default_project and comma separated list of bugs, yields bug line
   values.
@@ -3873,9 +3848,10 @@ def CMDstatus(parser, args):
   parser.add_option(
       '-j', '--maxjobs', action='store', type=int,
       help='The maximum number of jobs to use when retrieving review status')
-
-  _add_codereview_issue_select_options(
-    parser, 'Must be in conjunction with --field.')
+  parser.add_option(
+      '-i', '--issue', type=int,
+      help='Operate on this issue instead of the current branch\'s implicit '
+      'issue. Requires --field to be set.')
   options, args = parser.parse_args(args)
   if args:
     parser.error('Unsupported args: %s' % args)
@@ -4014,7 +3990,6 @@ def CMDissue(parser, args):
                          'issues will be listed.')
   parser.add_option('--json',
                     help='Path to JSON output file, or "-" for stdout.')
-  _add_codereview_select_options(parser)
   options, args = parser.parse_args(args)
 
   if options.reverse:
@@ -4030,12 +4005,11 @@ def CMDissue(parser, args):
       git_config[name] = val
 
     for branch in branches:
-      for cls in _CODEREVIEW_IMPLEMENTATIONS.values():
-        config_key = _git_branch_config_key(ShortBranchName(branch),
-                                            cls.IssueConfigKey())
-        issue = git_config.get(config_key)
-        if issue:
-          issue_branch_map.setdefault(int(issue), []).append(branch)
+      config_key = _git_branch_config_key(ShortBranchName(branch),
+                                          Changelist.IssueConfigKey())
+      issue = git_config.get(config_key)
+      if issue:
+        issue_branch_map.setdefault(int(issue), []).append(branch)
     if not args:
       args = sorted(issue_branch_map.iterkeys())
     result = {}
@@ -4086,7 +4060,6 @@ def CMDcomments(parser, args):
                          'editor parsing')
   parser.add_option('-j', '--json-file',
                     help='File to write JSON summary to, or "-" for stdout')
-  _add_codereview_select_options(parser)
   options, args = parser.parse_args(args)
 
   issue = None
@@ -4144,7 +4117,6 @@ def CMDdescription(parser, args):
                     help='Delete any unpublished Gerrit edits for this issue '
                          'without prompting')
 
-  _add_codereview_select_options(parser)
   options, args = parser.parse_args(args)
 
   target_issue_arg = None
@@ -4494,7 +4466,6 @@ def CMDupload(parser, args):
                          'or a new commit is created.')
 
   orig_args = args
-  _add_codereview_select_options(parser)
   (options, args) = parser.parse_args(args)
 
   if git_common.is_dirty_git_tree('upload'):
@@ -4662,7 +4633,6 @@ def CMDpatch(parser, args):
                     help='Performs a pull before reapplying.')
   parser.add_option_group(group)
 
-  _add_codereview_select_options(parser)
   (options, args) = parser.parse_args(args)
 
   if options.reapply:
@@ -4800,7 +4770,10 @@ def CMDtry(parser, args):
       '-R', '--retry-failed', action='store_true', default=False,
       help='Retry failed jobs from the latest set of tryjobs. '
            'Not allowed with --bucket and --bot options.')
-  _add_codereview_issue_select_options(parser)
+  parser.add_option(
+      '-i', '--issue', type=int,
+      help='Operate on this issue instead of the current branch\'s implicit '
+      'issue.')
   options, args = parser.parse_args(args)
 
   # Make sure that all properties are prop=value pairs.
@@ -4889,7 +4862,10 @@ def CMDtry_results(parser, args):
       '--json', help=('Path of JSON output file to write tryjob results to,'
                       'or "-" for stdout.'))
   parser.add_option_group(group)
-  _add_codereview_issue_select_options(parser)
+  parser.add_option(
+      '-i', '--issue', type=int,
+      help='Operate on this issue instead of the current branch\'s implicit '
+      'issue.')
   options, args = parser.parse_args(args)
   if args:
     parser.error('Unrecognized args: %s' % ' '.join(args))
@@ -4977,7 +4953,10 @@ def CMDset_commit(parser, args):
                     help='trigger in dry run mode')
   parser.add_option('-c', '--clear', action='store_true',
                     help='stop CQ run, if any')
-  _add_codereview_issue_select_options(parser)
+  parser.add_option(
+      '-i', '--issue', type=int,
+      help='Operate on this issue instead of the current branch\'s implicit '
+      'issue.')
   options, args = parser.parse_args(args)
   if args:
     parser.error('Unrecognized args: %s' % ' '.join(args))
@@ -5000,7 +4979,10 @@ def CMDset_commit(parser, args):
 @metrics.collector.collect_metrics('git cl set-close')
 def CMDset_close(parser, args):
   """Closes the issue."""
-  _add_codereview_issue_select_options(parser)
+  parser.add_option(
+      '-i', '--issue', type=int,
+      help='Operate on this issue instead of the current branch\'s implicit '
+      'issue.')
   options, args = parser.parse_args(args)
   if args:
     parser.error('Unrecognized args: %s' % ' '.join(args))
@@ -5457,17 +5439,16 @@ def CMDcheckout(parser, args):
 
   target_issue = str(issue_arg.issue)
 
-  def find_issues(issueprefix):
-    output = RunGit(['config', '--local', '--get-regexp',
-                     r'branch\..*\.%s' % issueprefix],
-                     error_ok=True)
-    for key, issue in [x.split() for x in output.splitlines()]:
-      if issue == target_issue:
-        yield re.sub(r'branch\.(.*)\.%s' % issueprefix, r'\1', key)
+  issueprefix = Changelist.IssueConfigKey()
+  output = RunGit(['config', '--local', '--get-regexp',
+                   r'branch\..*\.%s' % issueprefix],
+                   error_ok=True)
 
   branches = []
-  for cls in _CODEREVIEW_IMPLEMENTATIONS.values():
-    branches.extend(find_issues(cls.IssueConfigKey()))
+  for key, issue in [x.split() for x in output.splitlines()]:
+    if issue == target_issue:
+      branches.append(re.sub(r'branch\.(.*)\.%s' % issueprefix, r'\1', key))
+
   if len(branches) == 0:
     print('No branch found for issue %s.' % target_issue)
     return 1

tests/git_cl_test.py

@@ -1915,7 +1915,7 @@ class TestGitCl(TestCase):
       ((['git', 'config', 'branch.master.last-upload-hash', 'deadbeef'],), ''),
       ((['git', 'config', 'branch.master.gerritsquashhash', 'deadbeef'],), ''),
     ]
-    self.assertEqual(git_cl.main(['patch', '--gerrit', '123456', '--force']), 0)
+    self.assertEqual(git_cl.main(['patch', '123456', '--force']), 0)
 
   def test_patch_gerrit_guess_by_url(self):
     self.calls += self._get_gerrit_codereview_server_calls(
@@ -2192,7 +2192,7 @@ class TestGitCl(TestCase):
     self.mock(git_cl.sys, 'stderr', out)
 
     try:
-      self.assertEqual(git_cl.main(['status', '--issue', '1', '--gerrit']), 0)
+      self.assertEqual(git_cl.main(['status', '--issue', '1']), 0)
     except SystemExit as ex:
       self.assertEqual(ex.code, 2)
       self.assertRegexpMatches(out.getvalue(), r'--field must be specified')
@@ -2207,7 +2207,7 @@ class TestGitCl(TestCase):
 
     self.mock(git_cl.Changelist, 'GetDescription', assertIssue)
     self.assertEqual(
-      git_cl.main(['status', '--issue', '1', '--gerrit', '--field', 'desc']),
+      git_cl.main(['status', '--issue', '1', '--field', 'desc']),
       0)
     self.assertEqual(out.getvalue(), 'foobar\n')
 
@@ -2220,7 +2220,7 @@ class TestGitCl(TestCase):
     self.mock(git_cl.Changelist, 'GetDescription', assertIssue)
     self.mock(git_cl.Changelist, 'CloseIssue', lambda *_: None)
     self.assertEqual(
-      git_cl.main(['set-close', '--issue', '1', '--gerrit']), 0)
+      git_cl.main(['set-close', '--issue', '1']), 0)
 
   def test_description(self):
     out = StringIO()
@@ -2288,7 +2288,7 @@ class TestGitCl(TestCase):
         ((['git', 'config', 'rietveld.bug-prefix'],), CERR1),
         ((['git', 'config', 'core.editor'],), 'vi'),
     ]
-    self.assertEqual(0, git_cl.main(['description', '--gerrit']))
+    self.assertEqual(0, git_cl.main(['description']))
 
   def test_description_does_not_append_bug_line_if_fixed_is_present(self):
     current_desc = 'Some.\n\nFixed: 123\nChange-Id: xxx'
@@ -2316,7 +2316,7 @@ class TestGitCl(TestCase):
         ((['git', 'config', 'rietveld.bug-prefix'],), CERR1),
         ((['git', 'config', 'core.editor'],), 'vi'),
     ]
-    self.assertEqual(0, git_cl.main(['description', '--gerrit']))
+    self.assertEqual(0, git_cl.main(['description']))
 
   def test_description_set_stdin(self):
     out = StringIO()
@@ -2748,8 +2748,7 @@ class TestGitCl(TestCase):
         'msg', None),
        None),
     ]
-    self.assertEqual(0, git_cl.main(['comment', '--gerrit', '-i', '10',
-                                     '-a', 'msg']))
+    self.assertEqual(0, git_cl.main(['comment', '-i', '10', '-a', 'msg']))
 
   def test_git_cl_comments_fetch_gerrit(self):
     self.mock(sys, 'stdout', StringIO())