jdiegodcp / ramlfications

Python parser for RAML
https://ramlfications.readthedocs.org
Apache License 2.0
234 stars 50 forks source link

Add pep8 / flake8 config to setup.cfg #87

Closed PiDelport closed 8 years ago

PiDelport commented 8 years ago

This gives useful reporting when just running pep8 / flake8 directly.

codecov-io commented 8 years ago

Current coverage is 96.94%

Merging #87 into master will decrease coverage by -0.75% as of 1bc9f11

Powered by Codecov. Updated on successful CI builds.

econchick commented 8 years ago

Hmmm, setup.cfg is meant to supply parameters when using python setup.py directly, e.g. python setup.py build. You should invoke flake8 via tox, e.g. tox -e flake8 (and/or having your local (not committed to the repo) ~/.config/flake8 file). The [flake8] env in tox.ini file should have those configurations.

PiDelport commented 8 years ago

Right, the idea of this change is just to make pep8 / flake8 more useful by default, rather than having their project-specific configuration only apply when run via Tox. I usually prefer running flake8 directly, for interactive development, because tox -e flake8 is a bit heavyweight.

Both tools recommend putting their per-project configuration in either setup.cfg or tox.ini: I usually default to using setup.cfg, but tox.ini works just as well. Should I update the PR to put the configuration there?

econchick commented 8 years ago

@tardyp tox.ini should already have this.

I understand your interest in having something lightweight. So with that, I'd suggest just having a local flake8 config file.

PiDelport commented 8 years ago

A personal config doesn't help for these project-specific settings, though.

I update the PR to add [pep8] and [flake8] to tox.ini instead of just passing the configuration directly as command-line arguments. This works as before when running tox -e flake8, but now running pep8 and flake8 directly will also do the right thing, and use the project-specific settings.

Does that look good?

econchick commented 8 years ago

Awesome, that works for me! Thank you!

econchick commented 8 years ago

Damn I just realized that the actual commit message is now incorrect after I merged it. gahhh

PiDelport commented 8 years ago

Whoops, I should have updated it, sorry. :)

C'est la vie!