git-cl: Clean up a bit

Change-Id: I93809da721d410090e7ceb140cf5d9c4bded3744
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1765838
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
Reviewed-by: Andrii Shyshkalov <tandrii@google.com>

git_cl.py

@@ -996,24 +996,22 @@ class _CQState(object):
 
 
 class _ParsedIssueNumberArgument(object):
-  def __init__(self, issue=None, patchset=None, hostname=None, codereview=None):
+  def __init__(self, issue=None, patchset=None, hostname=None):
     self.issue = issue
     self.patchset = patchset
     self.hostname = hostname
-    assert codereview in (None, 'gerrit', 'rietveld')
-    self.codereview = codereview
 
   @property
   def valid(self):
     return self.issue is not None
 
 
-def ParseIssueNumberArgument(arg, codereview=None):
+def ParseIssueNumberArgument(arg):
   """Parses the issue argument and returns _ParsedIssueNumberArgument."""
   fail_result = _ParsedIssueNumberArgument()
 
   if arg.isdigit():
-    return _ParsedIssueNumberArgument(issue=int(arg), codereview=codereview)
+    return _ParsedIssueNumberArgument(issue=int(arg))
   if not arg.startswith('http'):
     return fail_result
 
@@ -1023,10 +1021,6 @@ def ParseIssueNumberArgument(arg, codereview=None):
   except ValueError:
     return fail_result
 
-  if codereview is not None:
-    parsed = _CODEREVIEW_IMPLEMENTATIONS[codereview].ParseIssueURL(parsed_url)
-    return parsed or fail_result
-
   return _GerritChangelistImpl.ParseIssueURL(parsed_url) or fail_result
 
 
@@ -1064,26 +1058,16 @@ _CommentSummary = collections.namedtuple(
 class Changelist(object):
   """Changelist works with one changelist in local branch.
 
-  Supports two codereview backends: Rietveld or Gerrit, selected at object
-  creation.
-
   Notes:
     * Not safe for concurrent multi-{thread,process} use.
     * Caches values from current branch. Therefore, re-use after branch change
       with great care.
   """
 
-  def __init__(self, branchref=None, issue=None, codereview=None, **kwargs):
+  def __init__(self, branchref=None, issue=None, **kwargs):
     """Create a new ChangeList instance.
 
-    If issue is given, the codereview must be given too.
-
-    If `codereview` is given, it must be 'rietveld' or 'gerrit'.
-    Otherwise, it's decided based on current configuration of the local branch,
-    with default being 'rietveld' for backwards compatibility.
-    See _load_codereview_impl for more details.
-
-    **kwargs will be passed directly to codereview implementation.
+    **kwargs will be passed directly to Gerrit implementation.
     """
     # Poke settings so we get the "configure your server" message if necessary.
     global settings
@@ -1091,9 +1075,6 @@ class Changelist(object):
       # Happens when git_cl.py is used as a utility library.
       settings = Settings()
 
-    if issue:
-      assert codereview, 'codereview must be known, if issue is known'
-
     self.branchref = branchref
     if self.branchref:
       assert branchref.startswith('refs/heads/')
@@ -1112,44 +1093,7 @@ class Changelist(object):
     self._remote = None
     self._cached_remote_url = (False, None)  # (is_cached, value)
 
-    self._codereview_impl = None
-    self._codereview = None
-    self._load_codereview_impl(codereview, **kwargs)
-    assert self._codereview_impl
-    assert self._codereview in _CODEREVIEW_IMPLEMENTATIONS
-
-  def _load_codereview_impl(self, codereview=None, **kwargs):
-    if codereview:
-      assert codereview in _CODEREVIEW_IMPLEMENTATIONS, (
-          'codereview {} not in {}'.format(codereview,
-                                           _CODEREVIEW_IMPLEMENTATIONS))
-      cls = _CODEREVIEW_IMPLEMENTATIONS[codereview]
-      self._codereview = codereview
-      self._codereview_impl = cls(self, **kwargs)
-      return
-
-    # Automatic selection based on issue number set for a current branch.
-    # Rietveld takes precedence over Gerrit.
-    assert not self.issue
-    # Whether we find issue or not, we are doing the lookup.
-    self.lookedup_issue = True
-    if self.GetBranch():
-      for codereview, cls in _CODEREVIEW_IMPLEMENTATIONS.iteritems():
-        issue = _git_get_branch_config_value(
-            cls.IssueConfigKey(), value_type=int, branch=self.GetBranch())
-        if issue:
-          self._codereview = codereview
-          self._codereview_impl = cls(self, **kwargs)
-          self.issue = int(issue)
-          return
-
-    # No issue is set for this branch, so default to gerrit.
-    return self._load_codereview_impl(
-        codereview='gerrit',
-        **kwargs)
-
-  def IsGerrit(self):
-    return self._codereview == 'gerrit'
+    self._codereview_impl = _GerritChangelistImpl(self, **kwargs)
 
   def GetCCList(self):
     """Returns the users cc'd on this CL.
@@ -1476,6 +1420,7 @@ class Changelist(object):
               'Removing it.')
         RunGit(['commit', '--amend', '-m',
                 git_footers.remove_footer(msg, 'Change-Id')])
+      self.lookedup_issue = True
       self.issue = None
       self.patchset = None
 
@@ -1571,7 +1516,7 @@ class Changelist(object):
     except presubmit_support.PresubmitFailure as e:
       DieWithError('%s\nMaybe your depot_tools is out of date?' % e)
 
-  def CMDPatchIssue(self, issue_arg, reject, nocommit, directory):
+  def CMDPatchIssue(self, issue_arg, nocommit):
     """Fetches and applies the issue patch from codereview to local branch."""
     if isinstance(issue_arg, (int, long)) or issue_arg.isdigit():
       parsed_issue_arg = _ParsedIssueNumberArgument(int(issue_arg))
@@ -1583,11 +1528,10 @@ class Changelist(object):
       DieWithError('Failed to parse issue argument "%s". '
                    'Must be an issue number or a valid URL.' % issue_arg)
     return self._codereview_impl.CMDPatchWithParsedIssue(
-        parsed_issue_arg, reject, nocommit, directory, False)
+        parsed_issue_arg, nocommit, False)
 
   def CMDUpload(self, options, git_diff_args, orig_args):
     """Uploads a change to codereview."""
-    assert self.IsGerrit()
     custom_cl_base = None
     if git_diff_args:
       custom_cl_base = base_branch = git_diff_args[0]
@@ -1803,18 +1747,12 @@ class _ChangelistCodereviewBase(object):
     """Returns the most recent patchset number from the codereview site."""
     raise NotImplementedError()
 
-  def CMDPatchWithParsedIssue(self, parsed_issue_arg, reject, nocommit,
-                              directory, force):
+  def CMDPatchWithParsedIssue(self, parsed_issue_arg, nocommit, force):
     """Fetches and applies the issue.
 
     Arguments:
       parsed_issue_arg: instance of _ParsedIssueNumberArgument.
-      reject: if True, reject the failed patch instead of switching to 3-way
-        merge. Rietveld only.
-      nocommit: do not commit the patch, thus leave the tree dirty. Rietveld
-        only.
-      directory: switch to directory before applying the patch. Rietveld only.
-      force: if true, overwrites existing local state.
+      nocommit: do not commit the patch, thus leave the tree dirty.
     """
     raise NotImplementedError()
 
@@ -2379,10 +2317,7 @@ class _GerritChangelistImpl(_ChangelistCodereviewBase):
         break
     return 0
 
-  def CMDPatchWithParsedIssue(self, parsed_issue_arg, reject, nocommit,
-                              directory, force):
-    assert not reject
-    assert not directory
+  def CMDPatchWithParsedIssue(self, parsed_issue_arg, nocommit, force):
     assert parsed_issue_arg.valid
 
     self._changelist.issue = parsed_issue_arg.issue
@@ -2471,8 +2406,7 @@ class _GerritChangelistImpl(_ChangelistCodereviewBase):
       return _ParsedIssueNumberArgument(
           issue=int(match.group(3)),
           patchset=int(match.group(5)) if match.group(5) else None,
-          hostname=parsed_url.netloc,
-          codereview='gerrit')
+          hostname=parsed_url.netloc)
     return None
 
   def _GerritCommitMsgHookCheck(self, offer_removal):
@@ -3039,28 +2973,17 @@ def _add_codereview_issue_select_options(parser, extra=""):
   parser.add_option('-i', '--issue', type=int, help=text)
 
 
-def _process_codereview_issue_select_options(parser, options):
-  _process_codereview_select_options(parser, options)
-  if options.issue is not None and not options.forced_codereview:
-    parser.error('--issue must be specified with either --rietveld or --gerrit')
-
-
 def _add_codereview_select_options(parser):
-  """Appends --gerrit and --rietveld options to force specific codereview."""
+  """Appends --gerrit option to force specific codereview."""
   parser.codereview_group = optparse.OptionGroup(
-      parser, 'EXPERIMENTAL! Codereview override options')
+      parser, 'DEPRECATED! 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')
+      help='Deprecated. Noop. Do not use.')
 
 
 def _process_codereview_select_options(parser, options):
-  if options.rietveld:
-    parser.error('--rietveld is no longer supported.')
   options.forced_codereview = None
   if options.gerrit:
     options.forced_codereview = 'gerrit'
@@ -4117,7 +4040,7 @@ def CMDstatus(parser, args):
   _add_codereview_issue_select_options(
     parser, 'Must be in conjunction with --field.')
   options, args = parser.parse_args(args)
-  _process_codereview_issue_select_options(parser, options)
+  _process_codereview_select_options(parser, options)
   if args:
     parser.error('Unsupported args: %s' % args)
   auth_config = auth.extract_auth_config_from_options(options)
@@ -4126,8 +4049,7 @@ def CMDstatus(parser, args):
     parser.error('--field must be specified with --issue.')
 
   if options.field:
-    cl = Changelist(auth_config=auth_config, issue=options.issue,
-                    codereview=options.forced_codereview)
+    cl = Changelist(auth_config=auth_config, issue=options.issue)
     if options.field.startswith('desc'):
       print(cl.GetDescription())
     elif options.field == 'id':
@@ -4297,15 +4219,15 @@ def CMDissue(parser, args):
     return 0
 
   if len(args) > 0:
-    issue = ParseIssueNumberArgument(args[0], options.forced_codereview)
+    issue = ParseIssueNumberArgument(args[0])
     if not issue.valid:
       DieWithError('Pass a url or number to set the issue, 0 to unset it, '
                    'or no argument to list it.\n'
                    'Maybe you want to run git cl status?')
-    cl = Changelist(codereview=issue.codereview)
+    cl = Changelist()
     cl.SetIssue(issue.issue)
   else:
-    cl = Changelist(codereview=options.forced_codereview)
+    cl = Changelist()
   print('Issue number: %s (%s)' % (cl.GetIssue(), cl.GetIssueURL()))
   if options.json:
     write_json(options.json, {
@@ -4323,8 +4245,7 @@ def CMDcomments(parser, args):
   parser.add_option('-p', '--publish', action='store_true',
                     help='marks CL as ready and sends comment to reviewers')
   parser.add_option('-i', '--issue', dest='issue',
-                    help='review issue id (defaults to current issue). '
-                         'If given, requires --rietveld or --gerrit')
+                    help='review issue id (defaults to current issue).')
   parser.add_option('-m', '--machine-readable', dest='readable',
                     action='store_false', default=True,
                     help='output comments in a format compatible with '
@@ -4344,10 +4265,7 @@ def CMDcomments(parser, args):
     except ValueError:
       DieWithError('A review issue ID is expected to be a number.')
 
-  cl = Changelist(issue=issue, codereview='gerrit', auth_config=auth_config)
-
-  if not cl.IsGerrit():
-    parser.error('Rietveld is not supported.')
+  cl = Changelist(issue=issue, auth_config=auth_config)
 
   if options.comment:
     cl.AddComment(options.comment, options.publish)
@@ -4402,22 +4320,19 @@ def CMDdescription(parser, args):
 
   target_issue_arg = None
   if len(args) > 0:
-    target_issue_arg = ParseIssueNumberArgument(args[0],
-                                                options.forced_codereview)
+    target_issue_arg = ParseIssueNumberArgument(args[0])
     if not target_issue_arg.valid:
       parser.error('Invalid issue ID or URL.')
 
   kwargs = {
     'auth_config': auth.extract_auth_config_from_options(options),
-    'codereview': options.forced_codereview,
   }
   detected_codereview_from_url = False
   if target_issue_arg:
     kwargs['issue'] = target_issue_arg.issue
     kwargs['codereview_host'] = target_issue_arg.hostname
-    if target_issue_arg.codereview and not options.forced_codereview:
+    if not args[0].isdigit() and not options.forced_codereview:
       detected_codereview_from_url = True
-      kwargs['codereview'] = target_issue_arg.codereview
 
   cl = Changelist(**kwargs)
   if not cl.GetIssue():
@@ -4425,8 +4340,7 @@ def CMDdescription(parser, args):
     DieWithError('This branch has no associated changelist.')
 
   if detected_codereview_from_url:
-    logging.info('canonical issue/change URL: %s (type: %s)\n',
-                 cl.GetIssueURL(), target_issue_arg.codereview)
+    logging.info('canonical issue/change URL: %s\n', cl.GetIssueURL())
 
   description = ChangeDescription(cl.GetDescription())
 
@@ -4445,8 +4359,7 @@ def CMDdescription(parser, args):
 
     description.set_description(text)
   else:
-    description.prompt(git_footer=cl.IsGerrit())
-
+    description.prompt()
   if cl.GetDescription().strip() != description.description:
     cl.UpdateDescription(description.description, force=options.force)
   return 0
@@ -4775,17 +4688,7 @@ def CMDupload(parser, args):
   # For sanity of test expectations, do this otherwise lazy-loading *now*.
   settings.GetIsGerrit()
 
-  cl = Changelist(auth_config=auth_config, codereview=options.forced_codereview)
-  if not cl.IsGerrit():
-    # Error out with instructions for repos not yet configured for Gerrit.
-    print('=====================================')
-    print('NOTICE: Rietveld is no longer supported. '
-          'You can upload changes to Gerrit with')
-    print('  git cl upload --gerrit')
-    print('or set Gerrit to be your default code review tool with')
-    print('  git config gerrit.host true')
-    print('=====================================')
-    return 1
+  cl = Changelist(auth_config=auth_config)
 
   return cl.CMDUpload(options, args, orig_args)
 
@@ -4870,9 +4773,6 @@ def CMDland(parser, args):
 
   cl = Changelist(auth_config=auth_config)
 
-  if not cl.IsGerrit():
-    parser.error('Rietveld is not supported.')
-
   if not cl.GetIssue():
     DieWithError('You must upload the change first to Gerrit.\n'
                  '  If you would rather have `git cl land` upload '
@@ -4889,14 +4789,8 @@ def CMDpatch(parser, args):
                     help='create a new branch off trunk for the patch')
   parser.add_option('-f', '--force', action='store_true',
                     help='overwrite state on the current or chosen branch')
-  parser.add_option('-d', '--directory', action='store', metavar='DIR',
-                    help='change to the directory DIR immediately, '
-                         'before doing anything else. Rietveld only.')
-  parser.add_option('--reject', action='store_true',
-                    help='failed patches spew .rej files rather than '
-                        'attempting a 3-way merge. Rietveld only.')
   parser.add_option('-n', '--no-commit', action='store_true', dest='nocommit',
-                    help='don\'t commit after patch applies. Rietveld only.')
+                    help='don\'t commit after patch applies.')
 
   group = optparse.OptionGroup(
       parser,
@@ -4925,8 +4819,7 @@ def CMDpatch(parser, args):
     if len(args) > 0:
       parser.error('--reapply implies no additional arguments.')
 
-    cl = Changelist(auth_config=auth_config,
-                    codereview=options.forced_codereview)
+    cl = Changelist(auth_config=auth_config)
     if not cl.GetIssue():
       parser.error('Current branch must have an associated issue.')
 
@@ -4938,26 +4831,22 @@ def CMDpatch(parser, args):
     if options.pull:
       RunGit(['pull'])
 
-    return cl.CMDPatchIssue(cl.GetIssue(), options.reject, options.nocommit,
-                            options.directory)
+    return cl.CMDPatchIssue(cl.GetIssue(), options.nocommit)
 
   if len(args) != 1 or not args[0]:
     parser.error('Must specify issue number or URL.')
 
-  target_issue_arg = ParseIssueNumberArgument(args[0],
-                                              options.forced_codereview)
+  target_issue_arg = ParseIssueNumberArgument(args[0])
   if not target_issue_arg.valid:
     parser.error('Invalid issue ID or URL.')
 
   cl_kwargs = {
       'auth_config': auth_config,
       'codereview_host': target_issue_arg.hostname,
-      'codereview': options.forced_codereview,
   }
   detected_codereview_from_url = False
-  if target_issue_arg.codereview and not options.forced_codereview:
+  if not args[0].isdigit() and not options.forced_codereview:
     detected_codereview_from_url = True
-    cl_kwargs['codereview'] = target_issue_arg.codereview
     cl_kwargs['issue'] = target_issue_arg.issue
 
   # We don't want uncommitted changes mixed up with the patch.
@@ -4972,19 +4861,11 @@ def CMDpatch(parser, args):
 
   cl = Changelist(**cl_kwargs)
 
-  if cl.IsGerrit():
-    if options.reject:
-      parser.error('--reject is not supported with Gerrit code review.')
-    if options.directory:
-      parser.error('--directory is not supported with Gerrit codereview.')
-
   if detected_codereview_from_url:
-    print('canonical issue/change URL: %s (type: %s)\n' %
-          (cl.GetIssueURL(), target_issue_arg.codereview))
+    print('canonical issue/change URL: %s\n' % cl.GetIssueURL())
 
-  return cl.CMDPatchWithParsedIssue(target_issue_arg, options.reject,
-                                    options.nocommit, options.directory,
-                                    options.force)
+  return cl.CMDPatchWithParsedIssue(
+      target_issue_arg, options.nocommit, options.force)
 
 
 def GetTreeStatus(url=None):
@@ -5076,7 +4957,7 @@ def CMDtry(parser, args):
   auth.add_auth_options(parser)
   _add_codereview_issue_select_options(parser)
   options, args = parser.parse_args(args)
-  _process_codereview_issue_select_options(parser, options)
+  _process_codereview_select_options(parser, options)
   auth_config = auth.extract_auth_config_from_options(options)
 
   if options.master and options.master.startswith('luci.'):
@@ -5090,14 +4971,12 @@ def CMDtry(parser, args):
   if args:
     parser.error('Unknown arguments: %s' % args)
 
-  cl = Changelist(auth_config=auth_config, issue=options.issue,
-                  codereview=options.forced_codereview)
+  cl = Changelist(auth_config=auth_config, issue=options.issue)
   if not cl.GetIssue():
     parser.error('Need to upload first.')
 
-  if cl.IsGerrit():
-    # HACK: warm up Gerrit change detail cache to save on RPCs.
-    cl._codereview_impl._GetChangeDetail(['DETAILED_ACCOUNTS', 'ALL_REVISIONS'])
+  # HACK: warm up Gerrit change detail cache to save on RPCs.
+  cl._codereview_impl._GetChangeDetail(['DETAILED_ACCOUNTS', 'ALL_REVISIONS'])
 
   error_message = cl.CannotTriggerTryJobReason()
   if error_message:
@@ -5154,14 +5033,12 @@ def CMDtry_results(parser, args):
   auth.add_auth_options(parser)
   _add_codereview_issue_select_options(parser)
   options, args = parser.parse_args(args)
-  _process_codereview_issue_select_options(parser, options)
+  _process_codereview_select_options(parser, options)
   if args:
     parser.error('Unrecognized args: %s' % ' '.join(args))
 
   auth_config = auth.extract_auth_config_from_options(options)
-  cl = Changelist(
-      issue=options.issue, codereview=options.forced_codereview,
-      auth_config=auth_config)
+  cl = Changelist(issue=options.issue, auth_config=auth_config)
   if not cl.GetIssue():
     parser.error('Need to upload first.')
 
@@ -5247,15 +5124,14 @@ def CMDset_commit(parser, args):
   auth.add_auth_options(parser)
   _add_codereview_issue_select_options(parser)
   options, args = parser.parse_args(args)
-  _process_codereview_issue_select_options(parser, options)
+  _process_codereview_select_options(parser, options)
   auth_config = auth.extract_auth_config_from_options(options)
   if args:
     parser.error('Unrecognized args: %s' % ' '.join(args))
   if options.dry_run and options.clear:
     parser.error('Only one of --dry-run and --clear are allowed.')
 
-  cl = Changelist(auth_config=auth_config, issue=options.issue,
-                  codereview=options.forced_codereview)
+  cl = Changelist(auth_config=auth_config, issue=options.issue)
   if options.clear:
     state = _CQState.NONE
   elif options.dry_run:
@@ -5274,12 +5150,11 @@ def CMDset_close(parser, args):
   _add_codereview_issue_select_options(parser)
   auth.add_auth_options(parser)
   options, args = parser.parse_args(args)
-  _process_codereview_issue_select_options(parser, options)
+  _process_codereview_select_options(parser, options)
   auth_config = auth.extract_auth_config_from_options(options)
   if args:
     parser.error('Unrecognized args: %s' % ' '.join(args))
-  cl = Changelist(auth_config=auth_config, issue=options.issue,
-                  codereview=options.forced_codereview)
+  cl = Changelist(auth_config=auth_config, issue=options.issue)
   # Ensure there actually is an issue to close.
   if not cl.GetIssue():
     DieWithError('ERROR: No issue to close.')
@@ -5683,7 +5558,7 @@ def GetDirtyMetricsDirs(diff_files):
 @subcommand.usage('<codereview url or issue id>')
 @metrics.collector.collect_metrics('git cl checkout')
 def CMDcheckout(parser, args):
-  """Checks out a branch associated with a given Rietveld or Gerrit issue."""
+  """Checks out a branch associated with a given Gerrit issue."""
   _, args = parser.parse_args(args)
 
   if len(args) != 1:

tests/git_cl_test.py

@@ -154,8 +154,7 @@ class SystemExitMock(Exception):
 
 class TestGitClBasic(unittest.TestCase):
   def test_get_description(self):
-    cl = git_cl.Changelist(issue=1, codereview='gerrit',
-                           codereview_host='host')
+    cl = git_cl.Changelist(issue=1, codereview_host='host')
     cl.description = 'x'
     cl.has_description = True
     cl._codereview_impl.FetchDescription = lambda *a, **kw: 'y'
@@ -164,8 +163,7 @@ class TestGitClBasic(unittest.TestCase):
     self.assertEquals(cl.GetDescription(), 'y')
 
   def test_description_footers(self):
-    cl = git_cl.Changelist(issue=1, codereview='gerrit',
-                           codereview_host='host')
+    cl = git_cl.Changelist(issue=1, codereview_host='host')
     cl.description = '\n'.join([
       'This is some message',
       '',
@@ -472,7 +470,7 @@ class TestGitClBasic(unittest.TestCase):
 
 class TestParseIssueURL(unittest.TestCase):
   def _validate(self, parsed, issue=None, patchset=None, hostname=None,
-                codereview=None, fail=False):
+                fail=False):
     self.assertIsNotNone(parsed)
     if fail:
       self.assertFalse(parsed.valid)
@@ -481,7 +479,6 @@ class TestParseIssueURL(unittest.TestCase):
     self.assertEqual(parsed.issue, issue)
     self.assertEqual(parsed.patchset, patchset)
     self.assertEqual(parsed.hostname, hostname)
-    self.assertEqual(parsed.codereview, codereview)
 
   def _run_and_validate(self, func, url, *args, **kwargs):
     result = func(urlparse.urlparse(url))
@@ -493,7 +490,7 @@ class TestParseIssueURL(unittest.TestCase):
   def test_gerrit(self):
     def test(url, *args, **kwargs):
       self._run_and_validate(git_cl._GerritChangelistImpl.ParseIssueURL, url,
-                             *args, codereview='gerrit', **kwargs)
+                             *args, **kwargs)
 
     test('http://chrome-review.source.com/c/123',
          123, None, 'chrome-review.source.com')
@@ -516,9 +513,7 @@ class TestParseIssueURL(unittest.TestCase):
 
   def test_ParseIssueNumberArgument(self):
     def test(arg, *args, **kwargs):
-      codereview_hint = kwargs.pop('hint', None)
-      self._validate(git_cl.ParseIssueNumberArgument(arg, codereview_hint),
-                     *args, **kwargs)
+      self._validate(git_cl.ParseIssueNumberArgument(arg), *args, **kwargs)
 
     test('123', 123)
     test('', fail=True)
@@ -527,17 +522,12 @@ class TestParseIssueURL(unittest.TestCase):
     test('123a', fail=True)
     test('ssh://chrome-review.source.com/#/c/123/4/', fail=True)
 
-    # Looks like Rietveld and Gerrit, but we should select Gerrit now
-    # w/ or w/o hint.
     test('https://codereview.source.com/123',
-         123, None, 'codereview.source.com', 'gerrit',
-         hint='gerrit')
-    test('https://codereview.source.com/123',
-         123, None, 'codereview.source.com', 'gerrit')
+         123, None, 'codereview.source.com')
 
     # Gerrrit.
     test('https://chrome-review.source.com/c/123/4',
-         123, 4, 'chrome-review.source.com', 'gerrit')
+         123, 4, 'chrome-review.source.com')
     test('https://chrome-review.source.com/bad/123/4', fail=True)
 
 
@@ -807,18 +797,31 @@ class TestGitCl(TestCase):
 
   @classmethod
   def _gerrit_ensure_auth_calls(
-      cls, issue=None, skip_auth_check=False, short_hostname='chromium'):
+      cls, issue=None, skip_auth_check=False, short_hostname='chromium',
+      custom_cl_base=None):
     cmd = ['git', 'config', '--bool', 'gerrit.skip-ensure-authenticated']
     if skip_auth_check:
       return [((cmd, ), 'true')]
 
     calls = [((cmd, ), CERR1)]
+
+    if custom_cl_base:
+      calls += [
+        ((['git', 'symbolic-ref', 'HEAD'],), 'master'),
+      ]
+
     calls.extend([
         ((['git', 'config', 'branch.master.merge'],), 'refs/heads/master'),
         ((['git', 'config', 'branch.master.remote'],), 'origin'),
         ((['git', 'config', 'remote.origin.url'],),
          'https://%s.googlesource.com/my/repo' % short_hostname),
     ])
+
+    calls += [
+      ((['git', 'config', 'branch.master.gerritissue'],),
+        CERR1 if issue is None else str(issue)),
+    ]
+
     if issue:
       calls.extend([
           ((['git', 'config', 'branch.master.gerritserver'],), CERR1),
@@ -830,11 +833,10 @@ class TestGitCl(TestCase):
                          fetched_status=None, other_cl_owner=None,
                          custom_cl_base=None, short_hostname='chromium'):
     calls = cls._is_gerrit_calls(True)
-    calls += [
-      ((['git', 'symbolic-ref', 'HEAD'],), 'master'),
-      ((['git', 'config', 'branch.master.gerritissue'],),
-        CERR1 if issue is None else str(issue)),
-    ]
+    if not custom_cl_base:
+      calls += [
+        ((['git', 'symbolic-ref', 'HEAD'],), 'master'),
+      ]
 
     if custom_cl_base:
       ancestor_revision = custom_cl_base
@@ -850,7 +852,8 @@ class TestGitCl(TestCase):
 
     # Calls to verify branch point is ancestor
     calls += cls._gerrit_ensure_auth_calls(
-        issue=issue, short_hostname=short_hostname)
+        issue=issue, short_hostname=short_hostname,
+        custom_cl_base=custom_cl_base)
 
     if issue:
       calls += [
@@ -1763,7 +1766,6 @@ class TestGitCl(TestCase):
       # These calls detect codereview to use.
       self.calls += [
         ((['git', 'symbolic-ref', 'HEAD'],), 'master'),
-        ((['git', 'config', 'branch.master.gerritissue'],), CERR1),
       ]
     if detect_gerrit_server:
       self.calls += self._get_gerrit_codereview_server_calls(
@@ -1914,7 +1916,6 @@ class TestGitCl(TestCase):
 
     self.calls = [
       ((['git', 'symbolic-ref', 'HEAD'],), 'master'),
-      ((['git', 'config', 'branch.master.gerritissue'],), CERR1),
       ((['git', 'config', 'branch.master.gerritserver'],), CERR1),
       ((['git', 'config', 'branch.master.merge'],), 'refs/heads/master'),
       ((['git', 'config', 'branch.master.remote'],), 'origin'),
@@ -1963,10 +1964,9 @@ class TestGitCl(TestCase):
     self.mock(git_cl, 'DieWithError',
               lambda msg, change=None: self._mocked_call(['DieWithError', msg]))
     self.calls = self._gerrit_ensure_auth_calls(skip_auth_check=skip_auth_check)
-    cl = git_cl.Changelist(codereview='gerrit')
+    cl = git_cl.Changelist()
     cl.branch = 'master'
     cl.branchref = 'refs/heads/master'
-    cl.lookedup_issue = True
     return cl
 
   def test_gerrit_ensure_authenticated_missing(self):
@@ -2031,7 +2031,7 @@ class TestGitCl(TestCase):
     ]
     self.mock(git_cl.gerrit_util, 'CookiesAuthenticator',
               CookiesAuthenticatorMockFactory(hosts_with_creds={}))
-    cl = git_cl.Changelist(codereview='gerrit')
+    cl = git_cl.Changelist()
     cl.branch = 'master'
     cl.branchref = 'refs/heads/master'
     cl.lookedup_issue = True
@@ -2086,7 +2086,7 @@ class TestGitCl(TestCase):
       self.assertEqual(git_cl.main(['status', '--issue', '1']), 0)
     except SystemExit as ex:
       self.assertEqual(ex.code, 2)
-      self.assertRegexpMatches(out.getvalue(), r'--issue must be specified')
+      self.assertRegexpMatches(out.getvalue(), r'--field must be specified')
 
     out = StringIO.StringIO()
     self.mock(git_cl.sys, 'stderr', out)
@@ -2205,13 +2205,10 @@ class TestGitCl(TestCase):
 
     self.calls = [
       ((['git', 'for-each-ref', '--format=%(refname)', 'refs/heads'],),
-         'refs/heads/master\nrefs/heads/foo\nrefs/heads/bar'),
-      ((['git', 'config', 'branch.master.gerritissue'],), '456'),
-      ((['git', 'config', 'branch.foo.gerritissue'],), CERR1),
-      ((['git', 'config', 'branch.bar.gerritissue'],), '789'),
+       'refs/heads/master\nrefs/heads/foo\nrefs/heads/bar'),
       ((['git', 'symbolic-ref', 'HEAD'],), 'master'),
       ((['git', 'tag', 'git-cl-archived-456-foo', 'foo'],), ''),
-      ((['git', 'branch', '-D', 'foo'],), ''),
+      ((['git', 'branch', '-D', 'foo'],), '')
     ]
 
     self.mock(git_cl, 'get_cl_statuses',
@@ -2227,7 +2224,6 @@ class TestGitCl(TestCase):
     self.calls = [
       ((['git', 'for-each-ref', '--format=%(refname)', 'refs/heads'],),
          'refs/heads/master'),
-      ((['git', 'config', 'branch.master.gerritissue'],), '1'),
       ((['git', 'symbolic-ref', 'HEAD'],), 'master'),
     ]
 
@@ -2243,9 +2239,6 @@ class TestGitCl(TestCase):
     self.calls = [
       ((['git', 'for-each-ref', '--format=%(refname)', 'refs/heads'],),
          'refs/heads/master\nrefs/heads/foo\nrefs/heads/bar'),
-      ((['git', 'config', 'branch.master.gerritissue'],), '456'),
-      ((['git', 'config', 'branch.foo.gerritissue'],), CERR1),
-      ((['git', 'config', 'branch.bar.gerritissue'],), '789'),
       ((['git', 'symbolic-ref', 'HEAD'],), 'master')
     ]
 
@@ -2263,9 +2256,6 @@ class TestGitCl(TestCase):
     self.calls = [
       ((['git', 'for-each-ref', '--format=%(refname)', 'refs/heads'],),
          'refs/heads/master\nrefs/heads/foo\nrefs/heads/bar'),
-      ((['git', 'config', 'branch.master.gerritissue'],), '1'),
-      ((['git', 'config', 'branch.foo.gerritissue'],), '456'),
-      ((['git', 'config', 'branch.bar.gerritissue'],), CERR1),
       ((['git', 'symbolic-ref', 'HEAD'],), 'master'),
       ((['git', 'branch', '-D', 'foo'],), '')
     ]
@@ -2283,7 +2273,6 @@ class TestGitCl(TestCase):
     self.mock(git_cl.sys, 'stdout', out)
     self.calls = [
         ((['git', 'symbolic-ref', 'HEAD'],), 'feature'),
-        ((['git', 'config', 'branch.feature.gerritissue'],), '123'),
         # Let this command raise exception (retcode=1) - it should be ignored.
         ((['git', 'config', '--unset', 'branch.feature.last-upload-hash'],),
          CERR1),
@@ -2303,7 +2292,6 @@ class TestGitCl(TestCase):
               lambda _: 'This is a description\n\nChange-Id: Ideadbeef')
     self.calls = [
         ((['git', 'symbolic-ref', 'HEAD'],), 'feature'),
-        ((['git', 'config', 'branch.feature.gerritissue'],), '123'),
         # Let this command raise exception (retcode=1) - it should be ignored.
         ((['git', 'config', '--unset', 'branch.feature.last-upload-hash'],),
          CERR1),
@@ -2484,7 +2472,7 @@ class TestGitCl(TestCase):
         ((['git', 'rev-parse', '--show-cdup'],), '../'),
         ((['abspath', '../'],), '/abs/git_repo_root'),
     ]
-    return git_cl.Changelist(codereview='gerrit', issue=123)
+    return git_cl.Changelist(issue=123)
 
   def test_GerritCommitMsgHookCheck_custom_hook(self):
     cl = self._common_GerritCommitMsgHookCheck()
@@ -2523,7 +2511,7 @@ class TestGitCl(TestCase):
       ((['git', 'config', 'branch.feature.gerritserver'],),
        'chromium-review.googlesource.com'),
     ]
-    cl = git_cl.Changelist(issue=123, codereview='gerrit')
+    cl = git_cl.Changelist(issue=123)
     cl._codereview_impl._GetChangeDetail = lambda _: {
       'labels': {},
       'current_revision': 'deadbeaf',
@@ -2690,10 +2678,10 @@ class TestGitCl(TestCase):
         (('GetChangeDetail', 'host', 'ab%2Frepo~2', []), 'b'),
         (('GetChangeDetail', 'host', 'ab%2Frepo~2', []), 'b2'),
     ]
-    cl1 = git_cl.Changelist(issue=1, codereview='gerrit')
+    cl1 = git_cl.Changelist(issue=1)
     cl1._cached_remote_url = (
         True, 'https://chromium.googlesource.com/a/my/repo.git/')
-    cl2 = git_cl.Changelist(issue=2, codereview='gerrit')
+    cl2 = git_cl.Changelist(issue=2)
     cl2._cached_remote_url = (
         True, 'https://chromium.googlesource.com/ab/repo')
     self.assertEqual(cl1._GetChangeDetail(), 'a')  # Miss.
@@ -2712,7 +2700,7 @@ class TestGitCl(TestCase):
         # no longer in cache.
         (('GetChangeDetail', 'host', 'repo~1', ['B']), 'b'),
     ]
-    cl = git_cl.Changelist(issue=1, codereview='gerrit')
+    cl = git_cl.Changelist(issue=1)
     cl._cached_remote_url = (True, 'https://chromium.googlesource.com/repo/')
     self.assertEqual(cl._GetChangeDetail(options=['C', 'A', 'B']), 'cab')
     self.assertEqual(cl._GetChangeDetail(options=['A', 'B', 'C']), 'cab')
@@ -2746,7 +2734,7 @@ class TestGitCl(TestCase):
     ]
 
     self._mock_gerrit_changes_for_detail_cache()
-    cl = git_cl.Changelist(issue=1, codereview='gerrit')
+    cl = git_cl.Changelist(issue=1)
     cl._cached_remote_url = (
         True, 'https://chromium.googlesource.com/a/my/repo.git/')
     self.assertEqual(cl.GetDescription(), 'desc1')
@@ -3002,7 +2990,7 @@ class TestGitCl(TestCase):
         disapproval=False, approval=False, sender=u'reviewer@example.com'),
     ]
     cl = git_cl.Changelist(
-        codereview='gerrit', issue=1, branchref='refs/heads/foo')
+        issue=1, branchref='refs/heads/foo')
     self.assertEqual(cl.GetCommentsSummary(), expected_comments_summary)
     self.mock(git_cl.Changelist, 'GetBranch', lambda _: 'foo')
     self.assertEqual(
@@ -3117,7 +3105,7 @@ class TestGitCl(TestCase):
          autogenerated=True, approval=False, disapproval=False)
     ]
     cl = git_cl.Changelist(
-        codereview='gerrit', issue=1, branchref='refs/heads/foo')
+        issue=1, branchref='refs/heads/foo')
     self.assertEqual(cl.GetCommentsSummary(), expected_comments_summary)
 
   def test_get_remote_url_with_mirror(self):
@@ -3143,7 +3131,7 @@ class TestGitCl(TestCase):
       ((['git', 'config', 'remote.origin.url'],),
        url),
     ]
-    cl = git_cl.Changelist(codereview='gerrit', issue=1)
+    cl = git_cl.Changelist(issue=1)
     self.assertEqual(cl.GetRemoteUrl(), url)
     self.assertEqual(cl.GetRemoteUrl(), url)  # Must be cached.
 
@@ -3171,7 +3159,7 @@ class TestGitCl(TestCase):
         'Remote "origin" for branch "/cache/this-dir-doesnt-exist" points to'
         ' "master", but it doesn\'t exist.'), None),
     ]
-    cl = git_cl.Changelist(codereview='gerrit', issue=1)
+    cl = git_cl.Changelist(issue=1)
     self.assertIsNone(cl.GetRemoteUrl())
 
   def test_get_remote_url_misconfigured_mirror(self):
@@ -3205,7 +3193,7 @@ class TestGitCl(TestCase):
               'branch': 'master'}
         ), None),
     ]
-    cl = git_cl.Changelist(codereview='gerrit', issue=1)
+    cl = git_cl.Changelist(issue=1)
     self.assertIsNone(cl.GetRemoteUrl())
 
   def test_gerrit_change_identifier_with_project(self):
@@ -3216,7 +3204,7 @@ class TestGitCl(TestCase):
       ((['git', 'config', 'remote.origin.url'],),
        'https://chromium.googlesource.com/a/my/repo.git/'),
     ]
-    cl = git_cl.Changelist(codereview='gerrit', issue=123456)
+    cl = git_cl.Changelist(issue=123456)
     self.assertEqual(cl._GerritChangeIdentifier(), 'my%2Frepo~123456')
 
   def test_gerrit_change_identifier_without_project(self):
@@ -3226,7 +3214,7 @@ class TestGitCl(TestCase):
       ((['git', 'config', 'branch.master.remote'],), 'origin'),
       ((['git', 'config', 'remote.origin.url'],), CERR1),
     ]
-    cl = git_cl.Changelist(codereview='gerrit', issue=123456)
+    cl = git_cl.Changelist(issue=123456)
     self.assertEqual(cl._GerritChangeIdentifier(), '123456')