-
-
Notifications
You must be signed in to change notification settings - Fork 33.7k
Travis CI: Add flake8 jobs for Python 2.7 and Python 3.7 #21953
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
Conversation
|
My gut feeling says that I’d prefer #21952 (but it’s nice to have two options laid out!). /cc @nodejs/build-files @nodejs/python |
|
I tend to like this one because the committer gets a "fast fail". |
refack
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AFAICT this change is unneeded if #21952 lands.
| - make -j2 V= | ||
| script: | ||
| - make -j2 test-ci | ||
| - os: linux |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be covered by the lint-ci job
| - flake8 --version | ||
| - EXCLUDE=./deps/npm/node_modules/node-gyp,./deps/v8,./src/noperfctr_macros.py,./src/notrace_macros.py,./tools | ||
| - flake8 . --count --exclude=${EXCLUDE} --select=E901,E999,F821,F822,F823 --show-source --statistics | ||
| - os: linux |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not needed ATM. There are many script that are incompatible anyway.
| - export CXX="ccache clang++ -Qunused-arguments" | ||
| - export CC="ccache clang -Qunused-arguments -Wno-unknown-warning-option" | ||
| - export JOBS=2 | ||
| - export JOBS=4 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should check why this was 2 before changing
|
@cclauss thanks again. |
|
Is there a way to notify the committer that their PR contains errors detected by https://ci.nodejs.org? My sense is that my PR could contain errors but Travis CI and GitHub would be reporting all green. |
|
@cclauss The Node.js CI jobs should be posting status back to the associated PR. |
An alternative approach to #21952 that runs flake8 in parallel Travis CI jobs on both Python 2.7 and Python 3.7. The Python 3.7 job is run in allow_failures mode.
See #21952 for more details on the --exclude and --select choices. A mix of these two PRs is also possible.
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes