Run flake8 before pytest on travis #12523
Conversation
04c8249 to
f4627e8
Compare
|
Well, actually it was not that simple: Travis - Customizing the build step:
|
| if [[ $RUN_FLAKE8 == 1 ]]; then | ||
| flake8 --statistics && echo "Flake8 passed without any issues!" | ||
| fi | ||
|
|
There was a problem hiding this comment.
Or just (approximately)
- |
([[ $RUN_FLAKE8 != 1 ]] || flake8 ... && echo "flake8 passed") &&
echo "Called with $PYTEST_ADDOPTS" &&
python -mpytest
?
Relying on the shell seems simpler than relying on travis' API...
There was a problem hiding this comment.
I'm just relying on the shell. .travis-script.sh is just a bash script no travis API involved.
I find it simpler to be able to use a multi-line script instead of trying to jam all the logic in a single expression.
There was a problem hiding this comment.
I personally prefer not splitting the CI scripts over multiple files.
Instead of putting everything into a single expression you can also use set -e or add || exit 1 after every (relevant) line.
There was a problem hiding this comment.
Also, if you really want to move it to its own script, I guess it should go to ci/travis-script.sh.
|
Not sure I agree with this idea in the first place. Fiddling around with flake8 just so I can see if a pr passes all the tests is backwards. After fiddling w flake8 I find it fails an obscure test then I have to refactor and then fiddle with flake8 again. |
|
Still testing ... |
a429c94 to
1d452a7
Compare
|
I agree with @jklymak, and am -1 on this change - it is good to get feedback on both PEP8 and tests at the same time, instead of having to get PEP8 right to see if code passes the tests. |
1d452a7 to
545544c
Compare
|
I argue the reverse, it's quite annoying to wait 20min for the tests just to see that there's a minor code style issue. Alternatively, is it possible to run flake8 in a separate env? |
|
Yes, we should be able to run flake8 separately from the rest of the tests. This is what is done on several major other FOSS of the scientific python community. |
|
@NelleV can you name aonther project that does this, so that I can lookup their configuration? |
|
Scikit-learn, but it's not at all set up the same way.
…On Sat, Oct 20, 2018, 12:22 PM Tim Hoffmann ***@***.***> wrote:
@NelleV <https://github.com/NelleV> can you name aonther project that
does this, so that I can lookup their configuration?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#12523 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AALR3vwBadEvRA_f4_qUrLIhJCKPPKmKks5um3ffgaJpZM4XbDYJ>
.
|
|
Closed in favor of #12708. |
PR Summary
flake8 is significantly faster than running the tests. Let's first check if the code is formally correct. It doesn't make much sense to do a lengthy test run just to fail later because flake8 is not satisfied.