Re: [PATCH 1/1] Travis CI: Lint for Python syntax errors and undefined names

From: SZEDER Gábor <szeder.dev@gmail.com>
Date: Jul 20, 2019, 12:51 UTC
Message-ID: <20190720125127.GK20404@szeder.dev>
In-Reply-To: <ad1fdb86ae42378d10584deb58115adf11de8ef7.1563545933.git.gitgitgadget@gmail.com>

On Fri, Jul 19, 2019 at 07:18:55AM -0700, cclauss via GitGitGadget wrote:

Show 10 quoted lines

> Several things were changed between Python 2 and Python 3.
> There are a few Python 3 incompatibilities to work on.
> Here we are making changes to make the code run on both Py2 and Py3.
> We are doing this because the end of life of Python 2 is in 167 days.
> We are using print() function because legacy print statements are syntax
> errors on Py3.
> reduce() was moved in Python 3 and raw_input() was removed so we make
> changes to avoid NameErrors being raised at runtime.
> We are also putting flake8 lint tests in place on Travis CI to avoid
> any backsliding on future pull requests.

It seems to me that this patch does too many things at once, and perhaps it would be better to split it into a couple of smaller patches that do only one thing, e.g.:

  - use print function instead of statement in 'hg-to-git' (which
    constitutes the bulk of this patch),
  - do the same in 'contrib/fast-import/import-zips.py'
  - import 'reduce' and fix 'raw_input' in 'git-p4'
  - and once all that is done and the linter runs clean, add the
    linter job to Travis CI.

This would ease the job of the reader, now and in the future, and it would better stand out that ...

> diff --git a/git-p4.py b/git-p4.py
> index 3991e7d1a7..9faee25db2 100755
> --- a/git-p4.py
> +++ b/git-p4.py

Show 5 quoted lines

> @@ -3968,6 +3970,7 @@ def renameBranch(self, branch_name):
>                  break
>  
>          if not found:
> +            sync = P4Sync()

... this change is not mentioned in the commit message.

Is this something the linter complains about? It doesn't look like a Python 2 vs. 3 compatibility fix to me, so it might deserve a dedicated patch.

Cc-ing Luke for this bit.

Show 5 quoted lines

>              sys.exit("gave up trying to rename existing branch {0}".format(sync.branch))
>  
>      def findLastP4Revision(self, starting_point):
> -- 
> gitgitgadget