Merge pull request googleapis#2486 from tseaver/pycodestyle-use_diff_base

tseaver · web-flow · commit 32cce9181476 · 2016-10-04T01:11:48.000-04:00
Use 'diff_base' for pycodestyle check, if set.
diff --git a/scripts/pycodestyle_on_repo.py b/scripts/pycodestyle_on_repo.py
@@ -23,14 +23,17 @@
 import subprocess
 import sys
 
+from script_utils import get_affected_files
+
 
 def main():
     """Run pycodestyle on all Python files in the repository."""
     git_root = subprocess.check_output(
         ['git', 'rev-parse', '--show-toplevel']).strip()
     os.chdir(git_root)
-    python_files = subprocess.check_output(['git', 'ls-files', '*py'])
-    python_files = python_files.strip().split()
+    candidates, _ = get_affected_files()
+    python_files = [
+        candidate for candidate in candidates if candidate.endswith('.py')]
 
     pycodestyle_command = ['pycodestyle'] + python_files
     status_code = subprocess.call(pycodestyle_command)
diff --git a/scripts/run_pylint.py b/scripts/run_pylint.py
@@ -29,11 +29,7 @@
 import subprocess
 import sys
 
-from script_utils import LOCAL_BRANCH_ENV
-from script_utils import LOCAL_REMOTE_ENV
-from script_utils import in_travis
-from script_utils import in_travis_pr
-from script_utils import travis_branch
+from script_utils import get_affected_files
 
 
 IGNORED_DIRECTORIES = [
@@ -141,62 +137,10 @@ def is_production_filename(filename):
     return 'test' not in filename and 'docs' not in filename
 
 
-def get_files_for_linting(allow_limited=True):
-    """Gets a list of files in the repository.
-
-    By default, returns all files via ``git ls-files``. However, in some cases
-    uses a specific commit or branch (a so-called diff base) to compare
-    against for changed files. (This requires ``allow_limited=True``.)
-
-    To speed up linting on Travis pull requests against master, we manually
-    set the diff base to the branch the pull request is against. We don't do
-    this on "push" builds since "master" will be the currently checked out
-    code. One could potentially use ${TRAVIS_COMMIT_RANGE} to find a diff base
-    but this value is not dependable.
-
-    To allow faster local ``tox`` runs, the local remote and local branch
-    environment variables can be set to specify a remote branch to diff
-    against.
-
-    :type allow_limited: bool
-    :param allow_limited: Boolean indicating if a reduced set of files can
-                          be used.
-
-    :rtype: pair
-    :returns: Tuple of the diff base using the the list of filenames to be
-              linted.
-    """
-    diff_base = None
-    if in_travis():
-        # In the case of a pull request into a branch, we want to
-        # diff against HEAD in that branch.
-        if in_travis_pr():
-            diff_base = travis_branch()
-    else:
-        # Only allow specified remote and branch in local dev.
-        remote = os.getenv(LOCAL_REMOTE_ENV)
-        branch = os.getenv(LOCAL_BRANCH_ENV)
-        if remote is not None and branch is not None:
-            diff_base = '%s/%s' % (remote, branch)
-
-    if diff_base is not None and allow_limited:
-        result = subprocess.check_output(['git', 'diff', '--name-only',
-                                          diff_base])
-        print('Using files changed relative to %s:' % (diff_base,))
-        print('-' * 60)
-        print(result.rstrip('\n'))  # Don't print trailing newlines.
-        print('-' * 60)
-    else:
-        print('Diff base not specified, listing all files in repository.')
-        result = subprocess.check_output(['git', 'ls-files'])
-
-    return result.rstrip('\n').split('\n'), diff_base
-
-
 def get_python_files(all_files=None):
     """Gets a list of all Python files in the repository that need linting.
 
-    Relies on :func:`get_files_for_linting()` to determine which files should
+    Relies on :func:`get_affected_files()` to determine which files should
     be considered.
 
     NOTE: This requires ``git`` to be installed and requires that this
@@ -210,7 +154,7 @@ def get_python_files(all_files=None):
               contains all production files, the next all test files.
     """
     if all_files is None:
-        all_files, diff_base = get_files_for_linting()
+        all_files, diff_base = get_affected_files()
 
     library_files = []
     non_library_files = []
diff --git a/scripts/script_utils.py b/scripts/script_utils.py
@@ -14,6 +14,8 @@
 
 """Common helpers for testing scripts."""
 
+from __future__ import print_function
+
 import os
 import subprocess
 
@@ -159,3 +161,55 @@ def get_changed_packages(blob_name1, blob_name2, package_list):
             result.add(file_root)
 
     return sorted(result)
+
+
+def get_affected_files(allow_limited=True):
+    """Gets a list of files in the repository.
+
+    By default, returns all files via ``git ls-files``. However, in some cases
+    uses a specific commit or branch (a so-called diff base) to compare
+    against for changed files. (This requires ``allow_limited=True``.)
+
+    To speed up linting on Travis pull requests against master, we manually
+    set the diff base to the branch the pull request is against. We don't do
+    this on "push" builds since "master" will be the currently checked out
+    code. One could potentially use ${TRAVIS_COMMIT_RANGE} to find a diff base
+    but this value is not dependable.
+
+    To allow faster local ``tox`` runs, the local remote and local branch
+    environment variables can be set to specify a remote branch to diff
+    against.
+
+    :type allow_limited: bool
+    :param allow_limited: Boolean indicating if a reduced set of files can
+                          be used.
+
+    :rtype: pair
+    :returns: Tuple of the diff base using the list of filenames to be
+              linted.
+    """
+    diff_base = None
+    if in_travis():
+        # In the case of a pull request into a branch, we want to
+        # diff against HEAD in that branch.
+        if in_travis_pr():
+            diff_base = travis_branch()
+    else:
+        # Only allow specified remote and branch in local dev.
+        remote = os.getenv(LOCAL_REMOTE_ENV)
+        branch = os.getenv(LOCAL_BRANCH_ENV)
+        if remote is not None and branch is not None:
+            diff_base = '%s/%s' % (remote, branch)
+
+    if diff_base is not None and allow_limited:
+        result = subprocess.check_output(['git', 'diff', '--name-only',
+                                          diff_base])
+        print('Using files changed relative to %s:' % (diff_base,))
+        print('-' * 60)
+        print(result.rstrip('\n'))  # Don't print trailing newlines.
+        print('-' * 60)
+    else:
+        print('Diff base not specified, listing all files in repository.')
+        result = subprocess.check_output(['git', 'ls-files'])
+
+    return result.rstrip('\n').split('\n'), diff_base
