[git-cl] Lint and clean-up git-cl, test, and related modules

In this CL:
 - Clarify some comments.
 - Remove some unused imports.
 - Make some style more consistent (e.g. quotes, whitespace)

Tools used: pyflakes, flake8 (most warnings ignored)

Change-Id: Ibfb6733c8d844b3c75a7f50b4f3c1d43afabb0ca
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1773856
Reviewed-by: Andrii Shyshkalov <tandrii@google.com>
Commit-Queue: Quinten Yearsley <qyearsley@chromium.org>

gerrit_client.py

@@ -19,14 +19,11 @@ import sys
 import urllib
 import urlparse
 
-from third_party import colorama
 import fix_encoding
 import gerrit_util
 import setup_color
 
 __version__ = '0.1'
-# Shortcut since it quickly becomes redundant.
-Fore = colorama.Fore
 
 
 def write_result(result, opt):
@@ -47,6 +44,7 @@ def CMDbranchinfo(parser, args):
   logging.info(result)
   write_result(result, opt)
 
+
 @subcommand.usage('[args ...]')
 def CMDbranch(parser, args):
   parser.add_option('--branch', dest='branch', help='branch name')
@@ -124,7 +122,7 @@ class OptionParser(optparse.OptionParser):
 def main(argv):
   if sys.hexversion < 0x02060000:
     print('\nYour python version %s is unsupported, please upgrade.\n'
-          %(sys.version.split(' ', 1)[0],),
+          % (sys.version.split(' ', 1)[0],),
           file=sys.stderr)
     return 2
   dispatcher = subcommand.CommandDispatcher(__name__)

gerrit_util.py

@@ -3,7 +3,7 @@
 # found in the LICENSE file.
 
 """
-Utilities for requesting information for a gerrit server via https.
+Utilities for requesting information for a Gerrit server via HTTPS.
 
 https://gerrit-review.googlesource.com/Documentation/rest-api.html
 """
@@ -44,7 +44,7 @@ LOGGER = logging.getLogger()
 TRY_LIMIT = 7
 
 
-# Controls the transport protocol used to communicate with gerrit.
+# Controls the transport protocol used to communicate with Gerrit.
 # This is parameterized primarily to enable GerritTestCase.
 GERRIT_PROTOCOL = 'https'
 
@@ -143,7 +143,7 @@ class CookiesAuthenticator(Authenticator):
   @classmethod
   def get_new_password_message(cls, host):
     if host is None:
-      return ('Git host for gerrit upload is unknown. Check your remote '
+      return ('Git host for Gerrit upload is unknown. Check your remote '
               'and the branch your branch is tracking. This tool assumes '
               'that you are using a git server at *.googlesource.com.')
     assert not host.startswith('http')
@@ -321,7 +321,6 @@ class GceAuthenticator(Authenticator):
         time.sleep(next_delay_sec)
         next_delay_sec *= 2
 
-
   @classmethod
   def _get_token_dict(cls):
     if cls._token_cache:
@@ -366,7 +365,7 @@ class LuciContextAuthenticator(Authenticator):
 
 
 def CreateHttpConn(host, path, reqtype='GET', headers=None, body=None):
-  """Opens an https connection to a gerrit service, and sends a request."""
+  """Opens an HTTPS connection to a Gerrit service, and sends a request."""
   headers = headers or {}
   bare_host = host.partition(':')[0]
 
@@ -407,7 +406,7 @@ def CreateHttpConn(host, path, reqtype='GET', headers=None, body=None):
 
 
 def ReadHttpResponse(conn, accept_statuses=frozenset([200])):
-  """Reads an http response from a connection into a string buffer.
+  """Reads an HTTP response from a connection into a string buffer.
 
   Args:
     conn: An Http object created by CreateHttpConn above.
@@ -445,8 +444,8 @@ def ReadHttpResponse(conn, accept_statuses=frozenset([200])):
       LOGGER.debug('got response %d for %s %s', response.status,
                    conn.req_params['method'], conn.req_params['uri'])
       # If 404 was in accept_statuses, then it's expected that the file might
-      # not exist, so don't return the gitiles error page because that's not the
-      # "content" that was actually requested.
+      # not exist, so don't return the gitiles error page because that's not
+      # the "content" that was actually requested.
       if response.status == 404:
         contents = ''
       break
@@ -511,6 +510,7 @@ def QueryChanges(host, params, first_param=None, limit=None, o_params=None,
     start: how many changes to skip (starting with the most recent)
     o_params: A list of additional output specifiers, as documented here:
         https://gerrit-review.googlesource.com/Documentation/rest-api-changes.html#list-changes
+
   Returns:
     A list of json-decoded query results.
   """
@@ -529,11 +529,11 @@ def QueryChanges(host, params, first_param=None, limit=None, o_params=None,
 
 def GenerateAllChanges(host, params, first_param=None, limit=500,
                        o_params=None, start=None):
-  """
-  Queries a gerrit-on-borg server for all the changes matching the query terms.
+  """Queries a gerrit-on-borg server for all the changes matching the query
+  terms.
 
   WARNING: this is unreliable if a change matching the query is modified while
-    this function is being called.
+  this function is being called.
 
   A single query to gerrit-on-borg is limited on the number of results by the
   limit parameter on the request (see QueryChanges) and the server maximum
@@ -549,6 +549,7 @@ def GenerateAllChanges(host, params, first_param=None, limit=500,
     A generator object to the list of returned changes.
   """
   already_returned = set()
+
   def at_most_once(cls):
     for cl in cls:
       if cl['_number'] not in already_returned:
@@ -615,12 +616,12 @@ def MultiQueryChanges(host, params, change_list, limit=None, o_params=None,
 
 
 def GetGerritFetchUrl(host):
-  """Given a gerrit host name returns URL of a gerrit instance to fetch from."""
+  """Given a Gerrit host name returns URL of a Gerrit instance to fetch from."""
   return '%s://%s/' % (GERRIT_PROTOCOL, host)
 
 
 def GetCodeReviewTbrScore(host, project):
-  """Given a gerrit host name and project, return the Code-Review score for TBR.
+  """Given a Gerrit host name and project, return the Code-Review score for TBR.
   """
   conn = CreateHttpConn(host, '/projects/%s' % urllib.quote(project, safe=''))
   project = ReadHttpJsonResponse(conn)
@@ -632,23 +633,23 @@ def GetCodeReviewTbrScore(host, project):
 
 
 def GetChangePageUrl(host, change_number):
-  """Given a gerrit host name and change number, return change page url."""
+  """Given a Gerrit host name and change number, returns change page URL."""
   return '%s://%s/#/c/%d/' % (GERRIT_PROTOCOL, host, change_number)
 
 
 def GetChangeUrl(host, change):
-  """Given a gerrit host name and change id, return an url for the change."""
+  """Given a Gerrit host name and change ID, returns a URL for the change."""
   return '%s://%s/a/changes/%s' % (GERRIT_PROTOCOL, host, change)
 
 
 def GetChange(host, change):
-  """Query a gerrit server for information about a single change."""
+  """Queries a Gerrit server for information about a single change."""
   path = 'changes/%s' % change
   return ReadHttpJsonResponse(CreateHttpConn(host, path))
 
 
 def GetChangeDetail(host, change, o_params=None):
-  """Query a gerrit server for extended information about a single change."""
+  """Queries a Gerrit server for extended information about a single change."""
   path = 'changes/%s/detail' % change
   if o_params:
     path += '?%s' % '&'.join(['o=%s' % p for p in o_params])
@@ -656,7 +657,7 @@ def GetChangeDetail(host, change, o_params=None):
 
 
 def GetChangeCommit(host, change, revision='current'):
-  """Query a gerrit server for a revision associated with a change."""
+  """Query a Gerrit server for a revision associated with a change."""
   path = 'changes/%s/revisions/%s/commit?links' % (change, revision)
   return ReadHttpJsonResponse(CreateHttpConn(host, path))
 
@@ -667,12 +668,12 @@ def GetChangeCurrentRevision(host, change):
 
 
 def GetChangeRevisions(host, change):
-  """Get information about all revisions associated with a change."""
+  """Gets information about all revisions associated with a change."""
   return QueryChanges(host, [], change, o_params=('ALL_REVISIONS',))
 
 
 def GetChangeReview(host, change, revision=None):
-  """Get the current review information for a change."""
+  """Gets the current review information for a change."""
   if not revision:
     jmsg = GetChangeRevisions(host, change)
     if not jmsg:
@@ -691,13 +692,13 @@ def GetChangeComments(host, change):
 
 
 def GetChangeRobotComments(host, change):
-  """Get the line- and file-level robot comments on a change."""
+  """Gets the line- and file-level robot comments on a change."""
   path = 'changes/%s/robotcomments' % change
   return ReadHttpJsonResponse(CreateHttpConn(host, path))
 
 
 def AbandonChange(host, change, msg=''):
-  """Abandon a gerrit change."""
+  """Abandons a Gerrit change."""
   path = 'changes/%s/abandon' % change
   body = {'message': msg} if msg else {}
   conn = CreateHttpConn(host, path, reqtype='POST', body=body)
@@ -705,7 +706,7 @@ def AbandonChange(host, change, msg=''):
 
 
 def RestoreChange(host, change, msg=''):
-  """Restore a previously abandoned change."""
+  """Restores a previously abandoned change."""
   path = 'changes/%s/restore' % change
   body = {'message': msg} if msg else {}
   conn = CreateHttpConn(host, path, reqtype='POST', body=body)
@@ -713,7 +714,7 @@ def RestoreChange(host, change, msg=''):
 
 
 def SubmitChange(host, change, wait_for_merge=True):
-  """Submits a gerrit change via Gerrit."""
+  """Submits a Gerrit change via Gerrit."""
   path = 'changes/%s/submit' % change
   body = {'wait_for_merge': wait_for_merge}
   conn = CreateHttpConn(host, path, reqtype='POST', body=body)
@@ -734,7 +735,7 @@ def HasPendingChangeEdit(host, change):
 
 def DeletePendingChangeEdit(host, change):
   conn = CreateHttpConn(host, 'changes/%s/edit' % change, reqtype='DELETE')
-  # On success, gerrit returns status 204; if the edit was already deleted it
+  # On success, Gerrit returns status 204; if the edit was already deleted it
   # returns 404.  Anything else is an error.
   ReadHttpResponse(conn, accept_statuses=[204, 404])
 
@@ -755,13 +756,13 @@ def SetCommitMessage(host, change, description, notify='ALL'):
 
 
 def GetReviewers(host, change):
-  """Get information about all reviewers attached to a change."""
+  """Gets information about all reviewers attached to a change."""
   path = 'changes/%s/reviewers' % change
   return ReadHttpJsonResponse(CreateHttpConn(host, path))
 
 
 def GetReview(host, change, revision):
-  """Get review information about a specific revision of a change."""
+  """Gets review information about a specific revision of a change."""
   path = 'changes/%s/revisions/%s/review' % (change, revision)
   return ReadHttpJsonResponse(CreateHttpConn(host, path))
 
@@ -810,7 +811,7 @@ def AddReviewers(host, change, reviewers=None, ccs=None, notify=True,
 
 
 def RemoveReviewers(host, change, remove=None):
-  """Remove reveiewers from a change."""
+  """Removes reviewers from a change."""
   if not remove:
     return
   if isinstance(remove, basestring):
@@ -828,7 +829,7 @@ def RemoveReviewers(host, change, remove=None):
 
 
 def SetReview(host, change, msg=None, labels=None, notify=None, ready=None):
-  """Set labels and/or add a message to a code review."""
+  """Sets labels and/or adds a message to a code review."""
   if not msg and not labels:
     return
   path = 'changes/%s/revisions/current/review' % change
@@ -853,7 +854,7 @@ def SetReview(host, change, msg=None, labels=None, notify=None, ready=None):
 
 def ResetReviewLabels(host, change, label, value='0', message=None,
                       notify=None):
-  """Reset the value of a given label for all reviewers on a change."""
+  """Resets the value of a given label for all reviewers on a change."""
   # This is tricky, because we want to work on the "current revision", but
   # there's always the risk that "current revision" will change in between
   # API calls.  So, we check "current revision" at the beginning and end; if
@@ -898,12 +899,12 @@ def ResetReviewLabels(host, change, label, value='0', message=None,
 
 
 def CreateGerritBranch(host, project, branch, commit):
-  """
-  Create a new branch from given project and commit
+  """Creates a new branch from given project and commit
+
   https://gerrit-review.googlesource.com/Documentation/rest-api-projects.html#create-branch
 
   Returns:
-    A JSON with 'ref' key
+    A JSON object with 'ref' key.
   """
   path = 'projects/%s/branches/%s' % (project, branch)
   body = {'revision': commit}
@@ -915,12 +916,13 @@ def CreateGerritBranch(host, project, branch, commit):
 
 
 def GetGerritBranch(host, project, branch):
-  """
-  Get a branch from given project and commit
+  """Gets a branch from given project and commit.
+
+  See:
   https://gerrit-review.googlesource.com/Documentation/rest-api-projects.html#get-branch
 
   Returns:
-    A JSON object with 'revision' key
+    A JSON object with 'revision' key.
   """
   path = 'projects/%s/branches/%s' % (project, branch)
   conn = CreateHttpConn(host, path, reqtype='GET')
@@ -937,7 +939,7 @@ def GetAccountDetails(host, account_id='self'):
   whichever account user is authenticating as.
 
   Documentation:
-    https://gerrit-review.googlesource.com/Documentation/rest-api-accounts.html#get-account
+  https://gerrit-review.googlesource.com/Documentation/rest-api-accounts.html#get-account
 
   Returns None if account is not found (i.e., Gerrit returned 404).
   """
@@ -955,11 +957,13 @@ def ValidAccounts(host, accounts, max_threads=10):
   accounts = list(set(accounts))
   if not accounts:
     return {}
+
   def get_one(account):
     try:
       return account, GetAccountDetails(host, account)
     except GerritError:
       return None, None
+
   valid = {}
   with contextlib.closing(ThreadPool(min(max_threads, len(accounts)))) as pool:
     for account, details in pool.map(get_one, accounts):
@@ -969,12 +973,11 @@ def ValidAccounts(host, accounts, max_threads=10):
 
 
 def PercentEncodeForGitRef(original):
-  """Apply percent-encoding for strings sent to gerrit via git ref metadata.
+  """Applies percent-encoding for strings sent to Gerrit via git ref metadata.
 
-  The encoding used is based on but stricter than URL encoding (Section 2.1
-  of RFC 3986). The only non-escaped characters are alphanumerics, and
-  'SPACE' (U+0020) can be represented as 'LOW LINE' (U+005F) or
-  'PLUS SIGN' (U+002B).
+  The encoding used is based on but stricter than URL encoding (Section 2.1 of
+  RFC 3986). The only non-escaped characters are alphanumerics, and 'SPACE'
+  (U+0020) can be represented as 'LOW LINE' (U+005F) or 'PLUS SIGN' (U+002B).
 
   For more information, see the Gerrit docs here:
 
@@ -983,7 +986,7 @@ def PercentEncodeForGitRef(original):
   safe = 'abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789 '
   encoded = ''.join(c if c in safe else '%%%02X' % ord(c) for c in original)
 
-  # spaces are not allowed in git refs; gerrit will interpret either '_' or
+  # Spaces are not allowed in git refs; gerrit will interpret either '_' or
   # '+' (or '%20') as space. Use '_' since that has been supported the longest.
   return encoded.replace(' ', '_')
 
@@ -1016,12 +1019,12 @@ assert all(3 == len(p) for p in _GERRIT_MIRROR_PREFIXES)
 
 
 def _UseGerritMirror(url, host):
-  """Returns new url which uses randomly selected mirror for a gerrit host.
+  """Returns a new URL which uses randomly selected mirror for a Gerrit host.
 
-  url's host should be for a given host or a result of prior call to this
+  The URL's host should be for a given host or a result of prior call to this
   function.
 
-  Assumes url has a single occurence of the host substring.
+  Assumes that the URL has a single occurence of the host substring.
   """
   assert host in url
   suffix = '-mirror-' + host
@@ -1032,7 +1035,7 @@ def _UseGerritMirror(url, host):
     actual_host = host
   else:
     # Already uses some mirror.
-    assert st >= prefix_len, (uri, host, st, prefix_len)
+    assert st >= prefix_len, (url, host, st, prefix_len)
     prefixes.remove(url[st-prefix_len:st])
     actual_host = url[st-prefix_len:st+len(suffix)]
   return url.replace(actual_host, random.choice(list(prefixes)) + suffix)

git_cl.py

@@ -15,7 +15,6 @@ import base64
 import collections
 import contextlib
 import datetime
-import fnmatch
 import httplib
 import itertools
 import json
@@ -42,11 +41,9 @@ from third_party import httplib2
 import auth
 import clang_format
 import dart_format
-import setup_color
 import fix_encoding
 import gclient_utils
 import gerrit_util
-import git_cache
 import git_common
 import git_footers
 import metrics
@@ -55,6 +52,7 @@ import owners
 import owners_finder
 import presubmit_support
 import scm
+import setup_color
 import split_cl
 import subcommand
 import subprocess2
@@ -112,10 +110,10 @@ DEFAULT_LINT_IGNORE_REGEX = r"$^"
 # File name for yapf style config files.
 YAPF_CONFIG_FILENAME = '.style.yapf'
 
-# Buildbucket master name prefix.
+# Buildbucket master name prefix for Buildbot masters.
 MASTER_PREFIX = 'master.'
 
-# Shortcut since it quickly becomes redundant.
+# Shortcut since it quickly becomes repetitive.
 Fore = colorama.Fore
 
 # Initialized in main()
@@ -194,7 +192,7 @@ def IsGitVersionAtLeast(min_version):
   prefix = 'git version '
   version = RunGit(['--version']).strip()
   return (version.startswith(prefix) and
-      LooseVersion(version[len(prefix):]) >= LooseVersion(min_version))
+          LooseVersion(version[len(prefix):]) >= LooseVersion(min_version))
 
 
 def BranchExists(branch):
@@ -276,7 +274,7 @@ def _git_get_branch_config_value(key, default=None, value_type=str,
   args = ['config']
   if value_type == bool:
     args.append('--bool')
-  # git config also has --int, but apparently git config suffers from integer
+  # `git config` also has --int, but apparently git config suffers from integer
   # overflows (http://crbug.com/640115), so don't use it.
   args.append(_git_branch_config_key(branch, key))
   code, out = RunGitWithCode(args)
@@ -307,8 +305,8 @@ def _git_set_branch_config_value(key, value, branch=None, **kwargs):
     args.append('--bool')
     value = str(value).lower()
   else:
-    # git config also has --int, but apparently git config suffers from integer
-    # overflows (http://crbug.com/640115), so don't use it.
+    # `git config` also has --int, but apparently git config suffers from
+    # integer overflows (http://crbug.com/640115), so don't use it.
     value = str(value)
   args.append(_git_branch_config_key(branch, key))
   if value is not None:
@@ -321,7 +319,7 @@ def _get_committer_timestamp(commit):
 
   Commit can be whatever git show would recognize, such as HEAD, sha1 or ref.
   """
-  # Git also stores timezone offset, but it only affects visual display,
+  # Git also stores timezone offset, but it only affects visual display;
   # actual point in time is defined by this timestamp only.
   return int(RunGit(['show', '-s', '--format=%ct', commit]).strip())
 
@@ -395,9 +393,9 @@ def _buildbucket_retry(operation_name, http, *args, **kwargs):
     if response.status == 200:
       if content_json is None:
         raise BuildbucketResponseException(
-            'Buildbucket returns invalid json content: %s.\n'
+            'Buildbucket returned invalid JSON content: %s.\n'
             'Please file bugs at http://crbug.com, '
-            'component "Infra>Platform>BuildBucket".' %
+            'component "Infra>Platform>Buildbucket".' %
             content)
       return content_json
     if response.status < 500 or try_count >= 2:
@@ -405,14 +403,14 @@ def _buildbucket_retry(operation_name, http, *args, **kwargs):
 
     # status >= 500 means transient failures.
     logging.debug('Transient errors when %s. Will retry.', operation_name)
-    time_sleep(0.5 + 1.5*try_count)
+    time_sleep(0.5 + (1.5 * try_count))
     try_count += 1
   assert False, 'unreachable'
 
 
 def _get_bucket_map(changelist, options, option_parser):
   """Returns a dict mapping bucket names to builders and tests,
-  for triggering try jobs.
+  for triggering tryjobs.
   """
   # If no bots are listed, we try to get a set of builders and tests based
   # on GetPreferredTryMasters functions in PRESUBMIT.py files.
@@ -443,11 +441,11 @@ def _get_bucket_map(changelist, options, option_parser):
 
 
 def _trigger_try_jobs(auth_config, changelist, buckets, options, patchset):
-  """Sends a request to Buildbucket to trigger try jobs for a changelist.
+  """Sends a request to Buildbucket to trigger tryjobs for a changelist.
 
   Args:
     auth_config: AuthConfig for Buildbucket.
-    changelist: Changelist that the try jobs are associated with.
+    changelist: Changelist that the tryjobs are associated with.
     buckets: A nested dict mapping bucket names to builders to tests.
     options: Command-line options.
   """
@@ -522,7 +520,7 @@ def _trigger_try_jobs(auth_config, changelist, buckets, options, patchset):
       )
 
   _buildbucket_retry(
-      'triggering try jobs',
+      'triggering tryjobs',
       http,
       buildbucket_put_url,
       'PUT',
@@ -536,7 +534,7 @@ def _trigger_try_jobs(auth_config, changelist, buckets, options, patchset):
 
 def fetch_try_jobs(auth_config, changelist, buildbucket_host,
                    patchset=None):
-  """Fetches try jobs from buildbucket.
+  """Fetches tryjobs from buildbucket.
 
   Returns a map from build id to build info as a dictionary.
   """
@@ -570,7 +568,7 @@ def fetch_try_jobs(auth_config, changelist, buildbucket_host,
     url = 'https://{hostname}/_ah/api/buildbucket/v1/search?{params}'.format(
         hostname=buildbucket_host,
         params=urllib.urlencode(params))
-    content = _buildbucket_retry('fetching try jobs', http, url, 'GET')
+    content = _buildbucket_retry('fetching tryjobs', http, url, 'GET')
     for build in content.get('builds', []):
       builds[build['id']] = build
     if 'next_cursor' in content:
@@ -583,7 +581,7 @@ def fetch_try_jobs(auth_config, changelist, buildbucket_host,
 def print_try_jobs(options, builds):
   """Prints nicely result of fetch_try_jobs."""
   if not builds:
-    print('No try jobs scheduled.')
+    print('No tryjobs scheduled.')
     return
 
   # Make a copy, because we'll be modifying builds dictionary.
@@ -676,7 +674,7 @@ def print_try_jobs(options, builds):
   pop(title='Other:',
       f=lambda b: (get_name(b), 'id=%s' % b['id']))
   assert len(builds) == 0
-  print('Total: %d try jobs' % total)
+  print('Total: %d tryjobs' % total)
 
 
 def _ComputeDiffLineRanges(files, upstream_commit):
@@ -734,8 +732,8 @@ def _FindYapfConfigFile(fpath, yapf_config_cache, top_dir=None):
   """Checks if a yapf file is in any parent directory of fpath until top_dir.
 
   Recursively checks parent directories to find yapf file and if no yapf file
-  is found returns None. Uses yapf_config_cache as a cache for
-  previously found configs.
+  is found returns None. Uses yapf_config_cache as a cache for previously found
+  configs.
   """
   fpath = os.path.abspath(fpath)
   # Return result if we've already computed it.
@@ -873,14 +871,14 @@ class Settings(object):
     return self._GetConfig('rietveld.cc', error_ok=True)
 
   def GetIsGerrit(self):
-    """Return true if this repo is associated with gerrit code review system."""
+    """Returns True if this repo is associated with Gerrit."""
     if self.is_gerrit is None:
       self.is_gerrit = (
           self._GetConfig('gerrit.host', error_ok=True).lower() == 'true')
     return self.is_gerrit
 
   def GetSquashGerritUploads(self):
-    """Return true if uploads to Gerrit should be squashed by default."""
+    """Returns True if uploads to Gerrit should be squashed by default."""
     if self.squash_gerrit_uploads is None:
       self.squash_gerrit_uploads = self.GetSquashGerritUploadsOverride()
       if self.squash_gerrit_uploads is None:
@@ -914,7 +912,7 @@ class Settings(object):
     return self.gerrit_skip_ensure_authenticated
 
   def GetGitEditor(self):
-    """Return the editor specified in the git config, or None if none is."""
+    """Returns the editor specified in the git config, or None if none is."""
     if self.git_editor is None:
       # Git requires single quotes for paths with spaces. We need to replace
       # them with double quotes for Windows to treat such paths as a single
@@ -1357,8 +1355,8 @@ class Changelist(object):
       self._cached_remote_url = (True, url)
       return url
 
-    # If it cannot be parsed as an url, assume it is a local directory, probably
-    # a git cache.
+    # If it cannot be parsed as an url, assume it is a local directory,
+    # probably a git cache.
     logging.warning('"%s" doesn\'t appear to point to a git host. '
                     'Interpreting it as a local directory.', url)
     if not os.path.isdir(url):
@@ -1428,7 +1426,7 @@ class Changelist(object):
     raw_description = self.GetDescription()
     msg_lines, _, footers = git_footers.split_footers(raw_description)
     if footers:
-      msg_lines = msg_lines[:len(msg_lines)-1]
+      msg_lines = msg_lines[:len(msg_lines) - 1]
     return msg_lines, footers
 
   def GetPatchset(self):
@@ -1601,10 +1599,9 @@ class Changelist(object):
       base_branch = self.GetCommonAncestorWithUpstream()
       git_diff_args = [base_branch, 'HEAD']
 
-
-    # Fast best-effort checks to abort before running potentially
-    # expensive hooks if uploading is likely to fail anyway. Passing these
-    # checks does not guarantee that uploading will not fail.
+    # Fast best-effort checks to abort before running potentially expensive
+    # hooks if uploading is likely to fail anyway. Passing these checks does
+    # not guarantee that uploading will not fail.
     self._codereview_impl.EnsureAuthenticated(force=options.force)
     self._codereview_impl.EnsureCanUploadPatchset(force=options.force)
 
@@ -1718,11 +1715,11 @@ class Changelist(object):
     return self._codereview_impl.GetMostRecentPatchset()
 
   def CannotTriggerTryJobReason(self):
-    """Returns reason (str) if unable trigger try jobs on this CL or None."""
+    """Returns reason (str) if unable trigger tryjobs on this CL or None."""
     return self._codereview_impl.CannotTriggerTryJobReason()
 
   def GetTryJobProperties(self, patchset=None):
-    """Returns dictionary of properties to launch try job."""
+    """Returns dictionary of properties to launch tryjob."""
     return self._codereview_impl.GetTryJobProperties(patchset=patchset)
 
   def __getattr__(self, attr):
@@ -1861,7 +1858,7 @@ class _ChangelistCodereviewBase(object):
     raise NotImplementedError()
 
   def CannotTriggerTryJobReason(self):
-    """Returns reason (str) if unable trigger try jobs on this CL or None."""
+    """Returns reason (str) if unable trigger tryjobs on this CL or None."""
     raise NotImplementedError()
 
   def GetIssueOwner(self):
@@ -1899,7 +1896,7 @@ class _GerritChangelistImpl(_ChangelistCodereviewBase):
       parsed = urlparse.urlparse(self.GetRemoteUrl())
       if parsed.scheme == 'sso':
         print('WARNING: using non-https URLs for remote is likely broken\n'
-              '  Your current remote is: %s'  % self.GetRemoteUrl())
+              '  Your current remote is: %s' % self.GetRemoteUrl())
         self._gerrit_host = '%s.googlesource.com' % self._gerrit_host
         self._gerrit_server = 'https://%s' % self._gerrit_host
     return self._gerrit_host
@@ -2075,7 +2072,7 @@ class _GerritChangelistImpl(_ChangelistCodereviewBase):
     return presubmit_support.GerritAccessor(self._GetGerritHost())
 
   def GetStatus(self):
-    """Apply a rough heuristic to give a simple summary of an issue's review
+    """Applies a rough heuristic to give a simple summary of an issue's review
     or CQ status, assuming adherence to a common workflow.
 
     Returns None if no issue for this branch, or one of the following keywords:
@@ -2327,7 +2324,7 @@ class _GerritChangelistImpl(_ChangelistCodereviewBase):
 
   def _IsCqConfigured(self):
     detail = self._GetChangeDetail(['LABELS'])
-    if not u'Commit-Queue' in detail.get('labels', {}):
+    if u'Commit-Queue' not in detail.get('labels', {}):
       return False
     # TODO(crbug/753213): Remove temporary hack
     if ('https://chromium.googlesource.com/chromium/src' ==
@@ -2482,8 +2479,8 @@ class _GerritChangelistImpl(_ChangelistCodereviewBase):
     hook = os.path.join(settings.GetRoot(), '.git', 'hooks', 'commit-msg')
     if not os.path.exists(hook):
       return
-    # Crude attempt to distinguish Gerrit Codereview hook from potentially
-    # custom developer made one.
+    # Crude attempt to distinguish Gerrit Codereview hook from a potentially
+    # custom developer-made one.
     data = gclient_utils.FileRead(hook)
     if not('From Gerrit Code Review' in data and 'add_ChangeId()' in data):
       return
@@ -2978,9 +2975,9 @@ class _GerritChangelistImpl(_ChangelistCodereviewBase):
   def SetCQState(self, new_state):
     """Sets the Commit-Queue label assuming canonical CQ config for Gerrit."""
     vote_map = {
-        _CQState.NONE:    0,
-        _CQState.DRY_RUN: 1,
-        _CQState.COMMIT:  2,
+      _CQState.NONE: 0,
+      _CQState.DRY_RUN: 1,
+      _CQState.COMMIT: 2,
     }
     labels = {'Commit-Queue': vote_map[new_state]}
     notify = False if new_state == _CQState.DRY_RUN else None
@@ -2998,7 +2995,7 @@ class _GerritChangelistImpl(_ChangelistCodereviewBase):
       return 'CL %s is closed' % self.GetIssue()
 
   def GetTryJobProperties(self, patchset=None):
-    """Returns dictionary of properties to launch try job."""
+    """Returns dictionary of properties to launch a tryjob."""
     data = self._GetChangeDetail(['ALL_REVISIONS'])
     patchset = int(patchset or self.GetPatchset())
     assert patchset
@@ -3420,7 +3417,7 @@ def FindCodereviewSettingsFile(filename='codereview.settings'):
 
 
 def LoadCodereviewSettingsFromFile(fileobj):
-  """Parse a codereview.settings file and updates hooks."""
+  """Parses a codereview.settings file and updates hooks."""
   keyvals = gclient_utils.ParseCodereviewSettingsContent(fileobj.read())
 
   def SetProperty(name, setting, unset_error_ok=False):
@@ -3463,8 +3460,10 @@ def LoadCodereviewSettingsFromFile(fileobj):
 
 
 def urlretrieve(source, destination):
-  """urllib is broken for SSL connections via a proxy therefore we
-  can't use urllib.urlretrieve()."""
+  """Downloads a network object to a local file, like urllib.urlretrieve.
+
+  This is necessary because urllib is broken for SSL connections via a proxy.
+  """
   with open(destination, 'w') as f:
     f.write(urllib2.urlopen(source).read())
 
@@ -3481,7 +3480,7 @@ def DownloadHooks(*args, **kwargs):
 
 
 def DownloadGerritHook(force):
-  """Download and install Gerrit commit-msg hook.
+  """Downloads and installs a Gerrit commit-msg hook.
 
   Args:
     force: True to update hooks. False to install hooks if not present.
@@ -3769,7 +3768,7 @@ class _GitCookiesChecker(object):
         found = True
         print('\n\n.gitcookies problem report:\n')
       bad_hosts.update(hosts or [])
-      print('  %s%s' % (title , (':' if sublines else '')))
+      print('  %s%s' % (title, (':' if sublines else '')))
       if sublines:
         print()
         print('    %s' % '\n    '.join(sublines))
@@ -3833,6 +3832,7 @@ def CMDbaseurl(parser, args):
     return RunGit(['config', 'branch.%s.base-url' % branch, args[0]],
                   error_ok=False).strip()
 
+
 def color_for_status(status):
   """Maps a Changelist status to color, for CMDstatus and other tools."""
   return {
@@ -3910,18 +3910,19 @@ def upload_branch_deps(cl, args):
   """Uploads CLs of local branches that are dependents of the current branch.
 
   If the local branch dependency tree looks like:
-  test1 -> test2.1 -> test3.1
-                   -> test3.2
-        -> test2.2 -> test3.3
+
+    test1 -> test2.1 -> test3.1
+                     -> test3.2
+          -> test2.2 -> test3.3
 
   and you run "git cl upload --dependencies" from test1 then "git cl upload" is
   run on the dependent branches in this order:
   test2.1, test3.1, test3.2, test2.2, test3.3
 
-  Note: This function does not rebase your local dependent branches. Use it when
-        you make a change to the parent branch that will not conflict with its
-        dependent branches, and you would like their dependencies updated in
-        Rietveld.
+  Note: This function does not rebase your local dependent branches. Use it
+        when you make a change to the parent branch that will not conflict
+        with its dependent branches, and you would like their dependencies
+        updated in Rietveld.
   """
   if git_common.is_dirty_git_tree('upload-branch-deps'):
     return 1
@@ -3941,8 +3942,8 @@ def upload_branch_deps(cl, args):
     print('No local branches found.')
     return 0
 
-  # Create a dictionary of all local branches to the branches that are dependent
-  # on it.
+  # Create a dictionary of all local branches to the branches that are
+  # dependent on it.
   tracked_to_dependents = collections.defaultdict(list)
   for b in branches.splitlines():
     tokens = b.split()
@@ -3953,6 +3954,7 @@ def upload_branch_deps(cl, args):
   print()
   print('The dependent local branches of %s are:' % root_branch)
   dependents = []
+
   def traverse_dependents_preorder(branch, padding=''):
     dependents_to_process = tracked_to_dependents.get(branch, [])
     padding += '  '
@@ -3960,6 +3962,7 @@ def upload_branch_deps(cl, args):
       print('%s%s' % (padding, dependent))
       dependents.append(dependent)
       traverse_dependents_preorder(dependent, padding)
+
   traverse_dependents_preorder(root_branch)
   print()
 
@@ -4557,14 +4560,14 @@ def CMDpresubmit(parser, args):
 
 
 def GenerateGerritChangeId(message):
-  """Returns Ixxxxxx...xxx change id.
+  """Returns the Change ID footer value (Ixxxxxx...xxx).
 
   Works the same way as
   https://gerrit-review.googlesource.com/tools/hooks/commit-msg
   but can be called on demand on all platforms.
 
-  The basic idea is to generate git hash of a state of the tree, original commit
-  message, author/committer info and timestamps.
+  The basic idea is to generate git hash of a state of the tree, original
+  commit message, author/committer info and timestamps.
   """
   lines = []
   tree_hash = RunGitSilent(['write-tree'])
@@ -4656,16 +4659,16 @@ def CMDupload(parser, args):
 
   Can skip dependency patchset uploads for a branch by running:
     git config branch.branch_name.skip-deps-uploads True
-  To unset run:
+  To unset, run:
     git config --unset branch.branch_name.skip-deps-uploads
   Can also set the above globally by using the --global flag.
 
-  If the name of the checked out branch starts with "bug-" or "fix-" followed by
-  a bug number, this bug number is automatically populated in the CL
+  If the name of the checked out branch starts with "bug-" or "fix-" followed
+  by a bug number, this bug number is automatically populated in the CL
   description.
 
   If subject contains text in square brackets or has "<text>: " prefix, such
-  text(s) is treated as Gerrit hashtags. For example, CLs with subjects
+  text(s) is treated as Gerrit hashtags. For example, CLs with subjects:
     [git-cl] add support for hashtags
     Foo bar: implement foo
   will be hashtagged with "git-cl" and "foo-bar" respectively.
@@ -4797,22 +4800,22 @@ def CMDsplit(parser, args):
   comment, the string '$directory', is replaced with the directory containing
   the shared OWNERS file.
   """
-  parser.add_option("-d", "--description", dest="description_file",
-                    help="A text file containing a CL description in which "
-                         "$directory will be replaced by each CL's directory.")
-  parser.add_option("-c", "--comment", dest="comment_file",
-                    help="A text file containing a CL comment.")
-  parser.add_option("-n", "--dry-run", dest="dry_run", action='store_true',
+  parser.add_option('-d', '--description', dest='description_file',
+                    help='A text file containing a CL description in which '
+                         '$directory will be replaced by each CL\'s directory.')
+  parser.add_option('-c', '--comment', dest='comment_file',
+                    help='A text file containing a CL comment.')
+  parser.add_option('-n', '--dry-run', dest='dry_run', action='store_true',
                     default=False,
-                    help="List the files and reviewers for each CL that would "
-                         "be created, but don't create branches or CLs.")
-  parser.add_option("--cq-dry-run", action='store_true',
-                    help="If set, will do a cq dry run for each uploaded CL. "
-                         "Please be careful when doing this; more than ~10 CLs "
-                         "has the potential to overload our build "
-                         "infrastructure. Try to upload these not during high "
-                         "load times (usually 11-3 Mountain View time). Email "
-                         "infra-dev@chromium.org with any questions.")
+                    help='List the files and reviewers for each CL that would '
+                         'be created, but don\'t create branches or CLs.')
+  parser.add_option('--cq-dry-run', action='store_true',
+                    help='If set, will do a cq dry run for each uploaded CL. '
+                         'Please be careful when doing this; more than ~10 CLs '
+                         'has the potential to overload our build '
+                         'infrastructure. Try to upload these not during high '
+                         'load times (usually 11-3 Mountain View time). Email '
+                         'infra-dev@chromium.org with any questions.')
   parser.add_option('-a', '--enable-auto-submit', action='store_true',
                     default=True,
                     help='Sends your change to the CQ after an approval. Only '
@@ -4895,7 +4898,6 @@ def CMDpatch(parser, args):
   parser.add_option('-n', '--no-commit', action='store_true', dest='nocommit',
                     help='don\'t commit after patch applies. Rietveld only.')
 
-
   group = optparse.OptionGroup(
       parser,
       'Options for continuing work on the current issue uploaded from a '
@@ -5029,8 +5031,8 @@ def CMDtree(parser, args):
 
 @metrics.collector.collect_metrics('git cl try')
 def CMDtry(parser, args):
-  """Triggers tryjobs using either BuildBucket or CQ dry run."""
-  group = optparse.OptionGroup(parser, 'Try job options')
+  """Triggers tryjobs using either Buildbucket or CQ dry run."""
+  group = optparse.OptionGroup(parser, 'Tryjob options')
   group.add_option(
       '-b', '--bot', action='append',
       help=('IMPORTANT: specify ONE builder per --bot flag. Use it multiple '
@@ -5046,7 +5048,7 @@ def CMDtry(parser, args):
       help=('DEPRECATED, use -B. The try master where to run the builds.'))
   group.add_option(
       '-r', '--revision',
-      help='Revision to use for the try job; default: the revision will '
+      help='Revision to use for the tryjob; default: the revision will '
            'be determined by the try recipe that builder runs, which usually '
            'defaults to HEAD of origin/master')
   group.add_option(
@@ -5065,8 +5067,8 @@ def CMDtry(parser, args):
       help='Specify generic properties in the form -p key1=value1 -p '
            'key2=value2 etc. The value will be treated as '
            'json if decodable, or as string otherwise. '
-           'NOTE: using this may make your try job not usable for CQ, '
-           'which will then schedule another try job with default properties')
+           'NOTE: using this may make your tryjob not usable for CQ, '
+           'which will then schedule another tryjob with default properties')
   group.add_option(
       '--buildbucket-host', default='cr-buildbucket.appspot.com',
       help='Host of buildbucket. The default host is %default.')
@@ -5099,7 +5101,7 @@ def CMDtry(parser, args):
 
   error_message = cl.CannotTriggerTryJobReason()
   if error_message:
-    parser.error('Can\'t trigger try jobs: %s' % error_message)
+    parser.error('Can\'t trigger tryjobs: %s' % error_message)
 
   if options.bucket and options.master:
     parser.error('Only one of --bucket and --master may be used.')
@@ -5134,7 +5136,7 @@ def CMDtry(parser, args):
 @metrics.collector.collect_metrics('git cl try-results')
 def CMDtry_results(parser, args):
   """Prints info about results for tryjobs associated with the current CL."""
-  group = optparse.OptionGroup(parser, 'Try job results options')
+  group = optparse.OptionGroup(parser, 'Tryjob results options')
   group.add_option(
       '-p', '--patchset', type=int, help='patchset number if not current.')
   group.add_option(
@@ -5146,7 +5148,7 @@ def CMDtry_results(parser, args):
       '--buildbucket-host', default='cr-buildbucket.appspot.com',
       help='Host of buildbucket. The default host is %default.')
   group.add_option(
-      '--json', help=('Path of JSON output file to write try job results to,'
+      '--json', help=('Path of JSON output file to write tryjob results to,'
                       'or "-" for stdout.'))
   parser.add_option_group(group)
   auth.add_auth_options(parser)
@@ -5220,8 +5222,8 @@ def CMDweb(parser, args):
     return 1
 
   # Redirect I/O before invoking browser to hide its output. For example, this
-  # allows to hide "Created new window in existing browser session." message
-  # from Chrome. Based on https://stackoverflow.com/a/2323563.
+  # allows us to hide the "Created new window in existing browser session."
+  # message from Chrome. Based on https://stackoverflow.com/a/2323563.
   saved_stdout = os.dup(1)
   saved_stderr = os.dup(2)
   os.close(1)
@@ -5638,13 +5640,13 @@ def CMDformat(parser, args):
         return_value = 2  # Not formatted.
       elif opts.diff and gn_ret == 2:
         # TODO this should compute and print the actual diff.
-        print("This change has GN build file diff for " + gn_diff_file)
+        print('This change has GN build file diff for ' + gn_diff_file)
       elif gn_ret != 0:
         # For non-dry run cases (and non-2 return values for dry-run), a
         # nonzero error code indicates a failure, probably because the file
         # doesn't parse.
-        DieWithError("gn format failed on " + gn_diff_file +
-                     "\nTry running 'gn format' on this file manually.")
+        DieWithError('gn format failed on ' + gn_diff_file +
+                     '\nTry running `gn format` on this file manually.')
 
   # Skip the metrics formatting from the global presubmit hook. These files have
   # a separate presubmit hook that issues an error if the files need formatting,
@@ -5664,13 +5666,15 @@ def CMDformat(parser, args):
 
   return return_value
 
+
 def GetDirtyMetricsDirs(diff_files):
   xml_diff_files = [x for x in diff_files if MatchingFileType(x, ['.xml'])]
   metrics_xml_dirs = [
     os.path.join('tools', 'metrics', 'actions'),
     os.path.join('tools', 'metrics', 'histograms'),
     os.path.join('tools', 'metrics', 'rappor'),
-    os.path.join('tools', 'metrics', 'ukm')]
+    os.path.join('tools', 'metrics', 'ukm'),
+  ]
   for xml_dir in metrics_xml_dirs:
     if any(file.startswith(xml_dir) for file in xml_diff_files):
       yield xml_dir
@@ -5782,7 +5786,7 @@ class OptionParser(optparse.OptionParser):
 
 def main(argv):
   if sys.hexversion < 0x02060000:
-    print('\nYour python version %s is unsupported, please upgrade.\n' %
+    print('\nYour Python version %s is unsupported, please upgrade.\n' %
           (sys.version.split(' ', 1)[0],), file=sys.stderr)
     return 2
 
@@ -5796,14 +5800,14 @@ def main(argv):
     if e.code != 500:
       raise
     DieWithError(
-        ('AppEngine is misbehaving and returned HTTP %d, again. Keep faith '
+        ('App Engine is misbehaving and returned HTTP %d, again. Keep faith '
          'and retry or visit go/isgaeup.\n%s') % (e.code, str(e)))
   return 0
 
 
 if __name__ == '__main__':
-  # These affect sys.stdout so do it outside of main() to simplify mocks in
-  # unit testing.
+  # These affect sys.stdout, so do it outside of main() to simplify mocks in
+  # the unit tests.
   fix_encoding.fix_encoding()
   setup_color.init()
   with metrics.collector.print_notice_and_exit():

gsutil.py

@@ -86,6 +86,7 @@ def temporary_directory(base):
     if os.path.isdir(tmpdir):
       shutil.rmtree(tmpdir)
 
+
 def ensure_gsutil(version, target, clean):
   bin_dir = os.path.join(target, 'gsutil_%s' % version)
   gsutil_bin = os.path.join(bin_dir, 'gsutil', 'gsutil')
@@ -183,5 +184,6 @@ def main():
   return run_gsutil(args.force_version, args.fallback, args.target, args.args,
                     clean=args.clean)
 
+
 if __name__ == '__main__':
   sys.exit(main())

tests/git_cl_test.py

@@ -5,7 +5,6 @@
 
 """Unit tests for git_cl.py."""
 
-import contextlib
 import datetime
 import json
 import logging
@@ -30,6 +29,7 @@ import git_common
 import git_footers
 import subprocess2
 
+
 def callError(code=1, cmd='', cwd='', stdout='', stderr=''):
   return subprocess2.CalledProcessError(code, cmd, cwd, stdout, stderr)
 
@@ -62,7 +62,7 @@ def MakeNamedTemporaryFileMock(expected_content):
 class ChangelistMock(object):
   # A class variable so we can access it when we don't have access to the
   # instance that's being set.
-  desc = ""
+  desc = ''
   def __init__(self, **kwargs):
     pass
   def GetIssue(self):
@@ -96,8 +96,8 @@ class CodereviewSettingsFileMock(object):
     pass
   # pylint: disable=no-self-use
   def read(self):
-    return ("CODE_REVIEW_SERVER: gerrit.chromium.org\n" +
-            "GERRIT_HOST: True\n")
+    return ('CODE_REVIEW_SERVER: gerrit.chromium.org\n' +
+            'GERRIT_HOST: True\n')
 
 
 class AuthenticatorMock(object):
@@ -224,7 +224,6 @@ class TestGitClBasic(unittest.TestCase):
       'Cq-Do-Not-Cancel-Tryjobs: true',
     ])
 
-
   def test_get_bug_line_values(self):
     f = lambda p, bugs: list(git_cl._get_bug_line_values(p, bugs))
     self.assertEqual(f('', ''), [])
@@ -471,7 +470,6 @@ class TestGitClBasic(unittest.TestCase):
     })
 
 
-
 class TestParseIssueURL(unittest.TestCase):
   def _validate(self, parsed, issue=None, patchset=None, hostname=None,
                 codereview=None, fail=False):
@@ -493,10 +491,6 @@ class TestParseIssueURL(unittest.TestCase):
     self._validate(result, *args, fail=False, **kwargs)
 
   def test_gerrit(self):
-    def test(url, issue=None, patchset=None, hostname=None, fail=None):
-      self._test_ParseIssueUrl(
-          git_cl._GerritChangelistImpl.ParseIssueURL,
-          url, issue, patchset, hostname, fail)
     def test(url, *args, **kwargs):
       self._run_and_validate(git_cl._GerritChangelistImpl.ParseIssueURL, url,
                              *args, codereview='gerrit', **kwargs)
@@ -650,7 +644,7 @@ class TestGitCl(TestCase):
     self.mock(git_common, 'is_dirty_git_tree', lambda x: False)
     self.mock(git_common, 'get_or_create_merge_base',
               lambda *a: (
-                  self._mocked_call(['get_or_create_merge_base']+list(a))))
+                  self._mocked_call(['get_or_create_merge_base'] + list(a))))
     self.mock(git_cl, 'BranchExists', lambda _: True)
     self.mock(git_cl, 'FindCodereviewSettingsFile', lambda: '')
     self.mock(git_cl, 'SaveDescriptionBackup', lambda _:
@@ -722,7 +716,7 @@ class TestGitCl(TestCase):
     # diagnose.
     if expected_args != args:
       N = 5
-      prior_calls  = '\n  '.join(
+      prior_calls = '\n  '.join(
           '@%d: %r' % (len(self._calls_done) - N + i, c[0])
           for i, c in enumerate(self._calls_done[-N:]))
       following_calls = '\n  '.join(
@@ -868,7 +862,7 @@ class TestGitCl(TestCase):
            'owner': {'email': (other_cl_owner or 'owner@example.com')},
            'change_id': '123456789',
            'current_revision': 'sha1_of_current_revision',
-           'revisions': { 'sha1_of_current_revision': {
+           'revisions': {'sha1_of_current_revision': {
              'commit': {'message': fetched_description},
            }},
            'status': fetched_status or 'NEW',
@@ -1383,8 +1377,8 @@ class TestGitCl(TestCase):
         squash_mode='override_nosquash',
         notify=True,
         change_id='I123456789',
-        final_description=
-            'desc\n\nBUG=\nR=foo@example.com\n\nChange-Id: I123456789')
+        final_description=(
+            'desc\n\nBUG=\nR=foo@example.com\n\nChange-Id: I123456789'))
 
   def test_gerrit_reviewer_multiple(self):
     self.mock(git_cl.gerrit_util, 'GetCodeReviewTbrScore',
@@ -1670,7 +1664,6 @@ class TestGitCl(TestCase):
           expected,
           'GetHashTags(%r) == %r, expected %r' % (desc, actual, expected))
 
-
     self.assertEqual(None, git_cl.GetTargetRef('origin', None, 'master'))
     self.assertEqual(None, git_cl.GetTargetRef(None,
                                                'refs/remotes/origin/master',
@@ -1719,7 +1712,7 @@ class TestGitCl(TestCase):
                                            branch))
 
   def test_patch_when_dirty(self):
-    # Patch when local tree is dirty
+    # Patch when local tree is dirty.
     self.mock(git_common, 'is_dirty_git_tree', lambda x: True)
     self.assertNotEqual(git_cl.main(['patch', '123456']), 0)
 
@@ -1740,12 +1733,12 @@ class TestGitCl(TestCase):
                     CERR1 if value is None else value))
     if value is None:
       calls += [
-          ((['git', 'config', 'branch.' + branch + '.merge'],),
-           'refs/heads' + branch),
-          ((['git', 'config', 'branch.' + branch + '.remote'],),
-           'origin'),
-          ((['git', 'config', 'remote.origin.url'],),
-           'https://%s.googlesource.com/my/repo' % git_short_host),
+        ((['git', 'config', 'branch.' + branch + '.merge'],),
+         'refs/heads' + branch),
+        ((['git', 'config', 'branch.' + branch + '.remote'],),
+         'origin'),
+        ((['git', 'config', 'remote.origin.url'],),
+         'https://%s.googlesource.com/my/repo' % git_short_host),
       ]
     return calls
 
@@ -1758,7 +1751,7 @@ class TestGitCl(TestCase):
     self.mock(git_cl, 'IsGitVersionAtLeast', lambda *args: True)
 
     if new_branch:
-      self.calls = [((['git', 'new-branch', 'master'],), ''),]
+      self.calls = [((['git', 'new-branch', 'master'],), '')]
 
     if codereview_in_url and actual_codereview == 'rietveld':
       self.calls += [
@@ -1914,6 +1907,7 @@ class TestGitCl(TestCase):
       git_cl.main(['patch', '123456'])
 
   def test_patch_gerrit_not_exists(self):
+
     def notExists(_issue, *_, **kwargs):
       raise git_cl.gerrit_util.GerritError(404, '')
     self.mock(git_cl.gerrit_util, 'GetChangeDetail', notExists)
@@ -2118,6 +2112,7 @@ class TestGitCl(TestCase):
     self.assertEqual(out.getvalue(), 'foobar\n')
 
   def test_SetCloseOverrideIssue(self):
+
     def assertIssue(cl_self, *_args):
       self.assertEquals(cl_self.issue, 1)
       return 'foobar'
@@ -2208,15 +2203,16 @@ class TestGitCl(TestCase):
   def test_archive(self):
     self.mock(git_cl.sys, 'stdout', StringIO.StringIO())
 
-    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'),
-         ((['git', 'tag', 'git-cl-archived-456-foo', 'foo'],), ''),
-         ((['git', 'branch', '-D', 'foo'],), '')]
+    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'),
+      ((['git', 'tag', 'git-cl-archived-456-foo', 'foo'],), ''),
+      ((['git', 'branch', '-D', 'foo'],), ''),
+    ]
 
     self.mock(git_cl, 'get_cl_statuses',
               lambda branches, fine_grained, max_processes:
@@ -2228,11 +2224,12 @@ class TestGitCl(TestCase):
 
   def test_archive_current_branch_fails(self):
     self.mock(git_cl.sys, 'stdout', StringIO.StringIO())
-    self.calls = \
-        [((['git', 'for-each-ref', '--format=%(refname)', 'refs/heads'],),
-          'refs/heads/master'),
-         ((['git', 'config', 'branch.master.gerritissue'],), '1'),
-         ((['git', 'symbolic-ref', 'HEAD'],), 'master')]
+    self.calls = [
+      ((['git', 'for-each-ref', '--format=%(refname)', 'refs/heads'],),
+         'refs/heads/master'),
+      ((['git', 'config', 'branch.master.gerritissue'],), '1'),
+      ((['git', 'symbolic-ref', 'HEAD'],), 'master'),
+    ]
 
     self.mock(git_cl, 'get_cl_statuses',
               lambda branches, fine_grained, max_processes:
@@ -2243,13 +2240,14 @@ class TestGitCl(TestCase):
   def test_archive_dry_run(self):
     self.mock(git_cl.sys, 'stdout', StringIO.StringIO())
 
-    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'),]
+    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')
+    ]
 
     self.mock(git_cl, 'get_cl_statuses',
               lambda branches, fine_grained, max_processes:
@@ -2262,14 +2260,15 @@ class TestGitCl(TestCase):
   def test_archive_no_tags(self):
     self.mock(git_cl.sys, 'stdout', StringIO.StringIO())
 
-    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'],), '')]
+    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'],), '')
+    ]
 
     self.mock(git_cl, 'get_cl_statuses',
               lambda branches, fine_grained, max_processes:
@@ -2337,7 +2336,7 @@ class TestGitCl(TestCase):
   def test_git_cl_try_default_cq_dry_run_gerrit(self):
     self.mock(git_cl.Changelist, 'GetChange',
               lambda _, *a: (
-                self._mocked_call(['GetChange']+list(a))))
+                self._mocked_call(['GetChange'] + list(a))))
     self.mock(git_cl.presubmit_support, 'DoGetTryMasters',
               lambda *_, **__: (
                 self._mocked_call(['DoGetTryMasters'])))
@@ -2360,7 +2359,7 @@ class TestGitCl(TestCase):
           'status': 'OPEN',
           'owner': {'email': 'owner@e.mail'},
           'revisions': {
-            'deadbeaf':  {
+            'deadbeaf': {
               '_number': 6,
             },
             'beeeeeef': {
@@ -2411,7 +2410,7 @@ class TestGitCl(TestCase):
           'status': 'OPEN',
           'owner': {'email': 'owner@e.mail'},
           'revisions': {
-            'deadbeaf':  {
+            'deadbeaf': {
               '_number': 6,
             },
             'beeeeeef': {
@@ -2663,7 +2662,7 @@ class TestGitCl(TestCase):
     # self.assertRegexpMatches(
     #     sys.stdout.getvalue(),
     #     r'Warning: Codereview server has newer patchsets \(13\)')
-    self.assertRegexpMatches(sys.stdout.getvalue(), 'No try jobs')
+    self.assertRegexpMatches(sys.stdout.getvalue(), 'No tryjobs')
 
   def test_fetch_try_jobs_some_gerrit(self):
     self._setup_fetch_try_jobs_gerrit({
@@ -2679,7 +2678,7 @@ class TestGitCl(TestCase):
     self.assertNotRegexpMatches(sys.stdout.getvalue(), 'Warning')
     self.assertRegexpMatches(sys.stdout.getvalue(), '^Failures:')
     self.assertRegexpMatches(sys.stdout.getvalue(), 'Started:')
-    self.assertRegexpMatches(sys.stdout.getvalue(), '2 try jobs')
+    self.assertRegexpMatches(sys.stdout.getvalue(), '2 tryjobs')
 
   def _mock_gerrit_changes_for_detail_cache(self):
     self.mock(git_cl._GerritChangelistImpl, '_GetGerritHost', lambda _: 'host')
@@ -2878,10 +2877,10 @@ class TestGitCl(TestCase):
         'owner': {'email': 'owner@example.com'},
         'current_revision': 'ba5eba11',
         'revisions': {
-          'deadbeaf':  {
+          'deadbeaf': {
             '_number': 1,
           },
-          'ba5eba11':  {
+          'ba5eba11': {
             '_number': 2,
           },
         },
@@ -2912,7 +2911,7 @@ class TestGitCl(TestCase):
           {
              u'_revision_number': 2,
              u'author': {
-               u'_account_id': 148512 ,
+               u'_account_id': 148512,
                u'email': u'reviewer@example.com',
                u'name': u'reviewer'
              },
@@ -2976,7 +2975,7 @@ class TestGitCl(TestCase):
           u'disapproval': False,
           u'sender': u'reviewer@example.com'
         }
-      ]),'')
+      ]), '')
     ]
     expected_comments_summary = [
       git_cl._CommentSummary(
@@ -3029,10 +3028,10 @@ class TestGitCl(TestCase):
         'owner': {'email': 'owner@example.com'},
         'current_revision': 'ba5eba11',
         'revisions': {
-          'deadbeaf':  {
+          'deadbeaf': {
             '_number': 1,
           },
-          'ba5eba11':  {
+          'ba5eba11': {
             '_number': 2,
           },
         },
@@ -3076,7 +3075,7 @@ class TestGitCl(TestCase):
           {
              u'_revision_number': 2,
              u'author': {
-               u'_account_id': 123 ,
+               u'_account_id': 123,
                u'email': u'tricium@serviceaccount.com',
                u'name': u'reviewer'
              },
@@ -3123,10 +3122,12 @@ class TestGitCl(TestCase):
 
   def test_get_remote_url_with_mirror(self):
     original_os_path_isdir = os.path.isdir
+
     def selective_os_path_isdir_mock(path):
       if path == '/cache/this-dir-exists':
         return self._mocked_call('os.path.isdir', path)
       return original_os_path_isdir(path)
+
     self.mock(os.path, 'isdir', selective_os_path_isdir_mock)
 
     url = 'https://chromium.googlesource.com/my/repo'
@@ -3148,10 +3149,12 @@ class TestGitCl(TestCase):
 
   def test_get_remote_url_non_existing_mirror(self):
     original_os_path_isdir = os.path.isdir
+
     def selective_os_path_isdir_mock(path):
       if path == '/cache/this-dir-doesnt-exist':
         return self._mocked_call('os.path.isdir', path)
       return original_os_path_isdir(path)
+
     self.mock(os.path, 'isdir', selective_os_path_isdir_mock)
     self.mock(logging, 'error',
               lambda fmt, *a: self._mocked_call('logging.error', fmt % a))
@@ -3173,10 +3176,12 @@ class TestGitCl(TestCase):
 
   def test_get_remote_url_misconfigured_mirror(self):
     original_os_path_isdir = os.path.isdir
+
     def selective_os_path_isdir_mock(path):
       if path == '/cache/this-dir-exists':
         return self._mocked_call('os.path.isdir', path)
       return original_os_path_isdir(path)
+
     self.mock(os.path, 'isdir', selective_os_path_isdir_mock)
     self.mock(logging, 'error',
               lambda *a: self._mocked_call('logging.error', *a))
@@ -3203,7 +3208,6 @@ class TestGitCl(TestCase):
     cl = git_cl.Changelist(codereview='gerrit', issue=1)
     self.assertIsNone(cl.GetRemoteUrl())
 
-
   def test_gerrit_change_identifier_with_project(self):
     self.calls = [
       ((['git', 'symbolic-ref', 'HEAD'],), 'master'),