From: "SZEDER Gábor" <szeder.dev@gmail.com>
To: cclauss via GitGitGadget <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org, Junio C Hamano <gitster@pobox.com>,
cclauss <cclauss@me.com>, Luke Diamand <luke@diamand.org>
Subject: Re: [PATCH 1/1] Travis CI: Lint for Python syntax errors and undefined names
Date: Sat, 20 Jul 2019 14:51:27 +0200 [thread overview]
Message-ID: <20190720125127.GK20404@szeder.dev> (raw)
In-Reply-To: <ad1fdb86ae42378d10584deb58115adf11de8ef7.1563545933.git.gitgitgadget@gmail.com>
On Fri, Jul 19, 2019 at 07:18:55AM -0700, cclauss via GitGitGadget wrote:
> 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
> @@ -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.
> sys.exit("gave up trying to rename existing branch {0}".format(sync.branch))
>
> def findLastP4Revision(self, starting_point):
> --
> gitgitgadget
prev parent reply other threads:[~2019-07-20 12:51 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-07-19 14:18 [PATCH 0/1] Travis CI: Lint for Python syntax errors and undefined names Christian Clauss via GitGitGadget
2019-07-19 14:18 ` [PATCH 1/1] " cclauss via GitGitGadget
2019-07-19 19:44 ` Junio C Hamano
2019-07-20 12:51 ` SZEDER Gábor [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190720125127.GK20404@szeder.dev \
--to=szeder.dev@gmail.com \
--cc=cclauss@me.com \
--cc=git@vger.kernel.org \
--cc=gitgitgadget@gmail.com \
--cc=gitster@pobox.com \
--cc=luke@diamand.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).