"cclauss via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:
> From: cclauss <cclauss@xxxxxx>
I'll tweak this line (and your sign-off) to read as "Christian
Clauss <cclauss@xxxxxx>" as you had in your cover letter.
> 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.
All sounds sensible, and the above is quite a good problem
description.
> 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.
Nothing is wrong here, but the convention in our codebase is to
describe the changes as if we are giving orders to the codebase "to
be like so". And as you have enumeration, I would write something
like this if I were describing this change:
- Use the `print()` function, because Py3 no longer has the `print`
statement.
- Import `reduce()` from functools, because Py3 requires this, and
importing also works with Py2 (even though it wasn't necessary).
- Use `input()` instead of `raw_input()`, as the former can be used
with both but the latter was removed in Py3.
Also add a CI job to Travis CI to run flake8 lint to avoid an
backsliding on future pull requests.
https://python-future.org/compatible_idioms.html#raw-input seems to
suggest, just like you import reduce from functools, you need to
import input from builtins. Is it not the case?
> + script: flake8 . --count --select=E9,F63,F72,F82 --show-source --statistics
How was the set of "select"ed violations to catch chosen? How are
we going to maintain this list going forward?
The rest of the patch looked sensible.
Thanks.
