Splitting trychange for git_try and removing no longer needed support for:

Splitting trychange for git_try and removing no longer needed support for:

SVN checkouts and
Try job submission via GIT/HTTP/Gerrit.

R=iannucci@chromium.org

[RobertoCN, 2015-03-13 20:33:19]

Does this look like what we discussed?

[iannucci, 2015-03-16 23:33:49]

looks a lot better... 1 megacomment. If you want to talk about it, feel free to
swing by my desk. If it's looking like a huge huge project, we can try tackling
the coding work here as a pair if that'll help.

https://codereview.chromium.org/1003733006/diff/20001/trychange_git.py
File trychange_git.py (right):

https://codereview.chromium.org/1003733006/diff/20001/trychange_git.py#newcod...
trychange_git.py:430: def _SendChangeSVN(bot_spec, options):
I would take the opportunity to break this out as well (probably into it's own
class or top-level function). Basically the way this all SHOULD have been
constructed is:

class LocalRepo(object):
  def __init__(self, cmdline_options):
    # defines interface of what PatchSinks can inquire about
    # from the local repo. To be implemented by PatchSources.

class PatchSource(object):
  def __init__(self, cmdline_options):
    # defines interface of how to get patch information. May also implement
    # LocalRepo.

class PatchSink(object):
  def __init__(self, cmdline_options, local_repo):
     # ... get stuff from cmdline & local_repo like svn url, etc.

  def send(patch_source):
     # actually send the patch from patch_source

class SVNSink(PatchSink):
  # implement sending a patch to SVN

## could also implement git, gerrit, etc.

class Git(LocalRepo, PatchSource):
  # it can answer questions from the local repo, and is a PatchSource


<cmdline --from=git --to=svn ...>
  Git(): <gather patch info>
    <get some info from cmdline>
    <get some from local repo (if insufficient info on commandline)>
  SvnSink: <send patch to svn>
    <get some info from cmdline>
    <maybe get some from LocalRepo object>

Does that make sense? that would also make all of this crap more testable (since
they'll all operate on the generic interface level of LocalRepo, PatchSource,
PatchSink, etc). You could also use that to make it so that each of the
Source/Sink classes can also define their own commandline options (e.g. settings
for a git sink won't make sense if you're sending to SVN, etc.)

Basically... trychange.py was written without a lot of thought :(.