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

Add pep8 exceptions for flake8 in setup.cfg #1636

Merged
merged 1 commit into from
Oct 24, 2017

Conversation

mcobzarenco
Copy link
Contributor

#10 suggests that the code style of gensim should ignore flake8's E501,E731,E12,W503,E402

This PR adds the rules to setup.cfg so they're documented and used by default.

@mcobzarenco mcobzarenco mentioned this pull request Oct 19, 2017
@menshikh-iv
Copy link
Contributor

menshikh-iv commented Oct 19, 2017

@menshikh-iv
Copy link
Contributor

ping @mcobzarenco

@mcobzarenco
Copy link
Contributor Author

Sorry for the delay -- ok, so:

  • I removed "--ignore ..." from the arguments of flake8 in flake8_diff.sh
  • I checked that flake8 uses the rules from setup.cfg when running flake8_diff.sh locally

@mcobzarenco
Copy link
Contributor Author

One thing to keep in mind with the current way of running flake8 (only run it on the files that were changed by a PR) is that it breaks down if one tries to change the flake8 rules and/or flake8 is updated.

One fix for the former would be for flake8_diff.sh to run flake on all files if it itself has been changed.

@menshikh-iv
Copy link
Contributor

@mcobzarenco please add --config argument with path explicitly for both calls.

@mcobzarenco
Copy link
Contributor Author

flake8 seems to look for setup.cfg / tox.ini / .flake8 in the root of the repo by default (https://travis-ci.org/RaRe-Technologies/gensim/jobs/292001367 seemed to have worked), but cool I also added an explicit --config cmd line argument, (I assume flake_diff.sh gets executed from the root of the repo by travis).

@menshikh-iv
Copy link
Contributor

Thanks, congratz with first contribution @mcobzarenco 👍

@menshikh-iv menshikh-iv merged commit 8097cad into piskvorky:develop Oct 24, 2017
@mcobzarenco
Copy link
Contributor Author

Thanks for merging!

horpto pushed a commit to horpto/gensim that referenced this pull request Oct 28, 2017
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.

2 participants