CI: Add TravisCI for code linting with flake8
#18
Conversation
Although this doesn't implement #16, it addresses the need for checking code style before merging in changes. This PR is not meant as an alternative to Black, but rather as an easy first step towards more comprehensive linting, while offloading the contributors from setting up anything on their end.
| @@ -90,9 +90,9 @@ parentdir_prefix = | |||
| [flake8] | |||
| max-line-length = 99 | |||
| doctests = True | |||
| exclude=*build/ | |||
| exclude=*build/,*/_afq/* | |||
There was a problem hiding this comment.
Is there still anything called *afq* anywhere in here?
There was a problem hiding this comment.
Yep, I placed the old code under dmriprep/workflows/_afq/ for when we reinclude stuff as an AFQ Plugin. Let me know if that's fine with you all.
|
|
||
| before_install: | ||
| - python -m pip install --upgrade pip | ||
| - pip install "flake8<3.0" flake8-putty |
There was a problem hiding this comment.
Might be worth adding a requirements-dev.txt file, specifying development requirements and installing here from that file, using pip install -r requirements-dev.txt?
There was a problem hiding this comment.
With the transition to PEP508 and others influencing the setup.cfg & setuptools mode of installation it is possible to minimize different distribution configurations - this covers developers and contributors if they install with pip install -e .[all].
For CI you are sometimes interested in avoiding the installation of the package (via pip, setup.py or wheels). We can try this and if it becomes burdensome to handle environments for Circle and/or Travis, then include a .maintenance/requirements-dev.txt file. WDYT?
|
Gotcha. Yes, that works. Is there relevant fmriprep documentation about
"plugins" and how those work?
…On Wed, Sep 25, 2019 at 3:44 PM Oscar Esteban ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In setup.cfg
<#18 (comment)>:
> @@ -90,9 +90,9 @@ parentdir_prefix =
[flake8]
max-line-length = 99
doctests = True
-exclude=*build/
+exclude=*build/,*/_afq/*
Yep, I placed the old code under dmriprep/workflows/_afq/ for when we
reinclude stuff as an *AFQ Plugin*. Let me know if that's fine with you
all.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#18?email_source=notifications&email_token=AAA46NVOBIDWQUWOJURHTXLQLPSWRA5CNFSM4I2SZP2KYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCF6PZNA#discussion_r328371098>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAA46NRJWVAADG5IQ22INX3QLPSWRANCNFSM4I2SZP2A>
.
|
That is an idea that sourced from @satra's brain and is gradually taking shape. In this case, I believe this "plugin" would be much easier if we just use BIDS-Derivatives as an interface since we would not need to modify dmriprep's execution graph, just add nodes. |
|
|
||
| before_install: | ||
| - python -m pip install --upgrade pip | ||
| - pip install "flake8<3.0" flake8-putty |
There was a problem hiding this comment.
If we want to use Black instead of flake8 as discussed in #16, then this line would be something like
- pip install black
| - pip install "flake8<3.0" flake8-putty | ||
|
|
||
| script: | ||
| - flake8 dmriprep |
There was a problem hiding this comment.
And if switching to black as discussed in #16, this line would read
- black --check .
|
I added a couple lines showing what changes would be necessary to switch to Black for formatting. But I don't think it's necessary to wait for a resolution of #16 to merge this PR. We can always switch over in the future. |
|
Yup, I can see how So I'd go ahead, and then add black in #16 (I like switching the pre-commit hook to optional). Possibly I would keep flake8 as enforced and allow the black tests to "fail" |
Although this doesn't implement #16, it addresses the need for checking code style before merging in changes. This PR is not meant as an alternative to Black, but rather as an easy first step towards more comprehensive linting, while offloading the contributors from setting up anything on their end.