Skip to content
This repository was archived by the owner on Nov 3, 2020. It is now read-only.

Conversation

@jbcrail
Copy link
Contributor

@jbcrail jbcrail commented Feb 17, 2017

No description provided.

@coveralls
Copy link

coveralls commented Feb 17, 2017

Coverage Status

Coverage decreased (-0.07%) to 73.981% when pulling c1f53c4 on jbcrail:run-flake8 into 073a9ec on dask:master.

@quasiben
Copy link
Member

@jbcrail do you think travis should fail if flake8 fails ?

@quasiben
Copy link
Member

Also, Thanks!

@mrocklin
Copy link
Member

Yeah, nice. A flake8 PR == <3 :)

@jbcrail
Copy link
Contributor Author

jbcrail commented Feb 21, 2017

@quasiben Yes, I'm partial to making Travis fail on any flake8 errors/warnings.

When resolving flake8 issues, I found an undefined method call (copy_) in commit b17ff99. The flake8 error uncovered an issue that would've arisen eventually (the method isn't invoked at this time).

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.1%) to 74.03% when pulling b77f4f9 on jbcrail:run-flake8 into e98f225 on dask:master.

@coveralls
Copy link

coveralls commented Feb 21, 2017

Coverage Status

Coverage increased (+0.4%) to 74.539% when pulling 035e259 on jbcrail:run-flake8 into e98f225 on dask:master.

@quasiben
Copy link
Member

@jbcrail are there other files you want to fix ? should we agree on some linting styles (column limits) and ignore versioneer.py ? I'm +1 on failing travis is linting fails, do you want to add that as well ? Happy to do all these thing but thought I'd ask since you are well into fixing these errors :)

@jbcrail
Copy link
Contributor Author

jbcrail commented Feb 23, 2017

@quasiben I'm finished updating the files now that flake8 passes.

I just noticed that you're using YAPF and certainly want to stick with the project's agreed-upon styling. I had originally set flake8 to use 119 (Django default), but I can update flake8 to use YAPF's defined column width of 120. I've already updated Travis to fail on flake8 errors/warnings. I also can update Travis to run flake8 only on the dask_ec2 directory.

After making these changes, should I run YAPF to format the code?

@coveralls
Copy link

coveralls commented Feb 24, 2017

Coverage Status

Coverage increased (+0.4%) to 74.539% when pulling e19f193 on jbcrail:run-flake8 into e98f225 on dask:master.

@coveralls
Copy link

coveralls commented Feb 24, 2017

Coverage Status

Coverage increased (+0.4%) to 74.539% when pulling e19f193 on jbcrail:run-flake8 into e98f225 on dask:master.

@quasiben
Copy link
Member

@jbcrail, apologies for letting this drag on. I think we should merge this and remove YAPF. Thank you so much for this contribution!

@quasiben
Copy link
Member

merging

@quasiben quasiben merged commit 660bbac into dask:master Mar 25, 2017
@jbcrail jbcrail deleted the run-flake8 branch March 30, 2017 21:10
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants