Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Run flake8 on Python instead of legacy Python #7388

Merged
merged 3 commits into from
Jun 27, 2019

Conversation

cclauss
Copy link
Contributor

@cclauss cclauss commented Jun 24, 2019

Blocked by: #7383, #7384, and #7385

Related to: #4552

Signed-off-by: cclauss cclauss@me.com

Also adds more critical flake8 tests.

For an explanation of how to fill out the fields, please see the relevant section
in PULL_REQUESTS.md

Description:
Risk Level:
Testing:
Docs Changes:
Release Notes:
[Optional Fixes #Issue]
[Optional Deprecated:]

@cclauss cclauss force-pushed the run-flake8-on-Python branch 4 times, most recently from b36795f to 9a97262 Compare June 24, 2019 18:33

echo "Running Python3 flake8 check..."
flake8 . --exclude=*/venv/* --count --select=E901,E999,F821,F822,F823 --show-source --statistics
python3 -m flake8 --version
python3 -m flake8 . --exclude=*/venv/* --count --select=E9,F63,F72,F82 --show-source --statistics
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we preserving semantics here (roughly)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. This is a superset of the previous tests: E9 runs all E9xx tests and F82 runs all F82x tests. We are forcing flake8 to run on Python 3 because it has a stricter syntax than legacy Python.

@htuch
Copy link
Member

htuch commented Jun 25, 2019

@cclauss this is failing the format check, can you take a look?
/wait

@cclauss
Copy link
Contributor Author

cclauss commented Jun 26, 2019

@htuch The commit message is clear: Blocked by: #7383, #7384, and #7385

@htuch
Copy link
Member

htuch commented Jun 26, 2019

@cclauss ideally put PRs into a draft status in this situation, it makes it easier to track what's ready for review, see https://github.blog/2019-02-14-introducing-draft-pull-requests/.

Signed-off-by: cclauss <cclauss@me.com>
Signed-off-by: cclauss <cclauss@me.com>
@asraa
Copy link
Contributor

asraa commented Jun 26, 2019

Hey, thanks! Do you mind also updated the python version in

virtualenv "${VENV_DIR}"/venv --no-site-packages --python=python2.7
?

And link issue #7322

@htuch
Copy link
Member

htuch commented Jun 26, 2019

/wait

Signed-off-by: cclauss <cclauss@me.com>
@cclauss
Copy link
Contributor Author

cclauss commented Jun 26, 2019

@asraa I am not sure how to "link issue #7322".

@htuch
Copy link
Member

htuch commented Jun 27, 2019

@cclauss I think the ask was to do this in the commit, I'll do that when I merge.

Copy link
Member

@htuch htuch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

@htuch htuch merged commit eb65b20 into envoyproxy:master Jun 27, 2019
@cclauss cclauss deleted the run-flake8-on-Python branch June 27, 2019 14:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants