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

Cleanup Travis and tox configs #1

Merged
merged 8 commits into from
Feb 9, 2018

Conversation

moggers87
Copy link
Collaborator

Cleaned up the Travis and tox configs, including some bits missed in my PR to django-sudo:

  • Moved the environment list completely to tox, which should be easier to read given how we need to exclude quite a few Django/Python combinations
  • Added linting and doc building to tox and fixed a few issues
  • Linting and doc building are done as separate jobs on Travis
  • Kept all this compatible with the already defined invoke tasks
  • Removed dependencies from dev-requirements.txt that are no longer needed

…by keeping the envlist in tox and using tox-travis to run tests on the
CI. A lot more concise than that long list of excludes we had before!
flake8 and sphinx will be installed by tox when required
@coveralls
Copy link

coveralls commented Jan 17, 2018

Coverage Status

Coverage remained the same at 100.0% when pulling 0a968a6 on moggers87:cleanup-old-cruft into 2a85095 on justinmayer:master.

Copy link
Owner

@justinmayer justinmayer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work, Matt. The switch to tox-travis is a big improvement. Other than my question about py.test, this all looks good. Looking forward to merging it.

@@ -1,9 +1,6 @@
-e .
flake8<3.0
invoke>=0.11.1,<0.12
pytest
pytest-cov
pytest-django
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think we need to remove the pytest packages here? Sometimes it's useful to do a quick test run on the current Python interpreter. If you still think we should remove these, then the reference to py.test should probably be removed from the Contributing > Running Tests docs.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I'll add it back in. I've since tried to do things like tox -e py27 and found that it does not work as I had assumed.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, cool. Might consider doing the same with flake8, since we presumably want to encourage folks to use it. 😉

Running tox can be cumbersome, so allow developers to use flake8 and
pytest outside of tox.

Added django to dev-requirements.txt too otherwise pytest will error.
@moggers87
Copy link
Collaborator Author

I've added flake8 and pytest back into dev-requirements.txt. I've also added django to that file because pytest is pretty useless without it.

@justinmayer
Copy link
Owner

Looks good to me. Do you think any of these commits should be squashed? I'm happy to combine them into a single commit upon merge, of course... Just wasn't sure if you wanted to organize them in any particular semantic manner. ☺️

@moggers87
Copy link
Collaborator Author

I have no personal preference, so feel free to do a squash-merge ☺

@justinmayer
Copy link
Owner

Many thanks for enhancements, Matt. 👍

@justinmayer justinmayer merged commit 8b233e6 into justinmayer:master Feb 9, 2018
@moggers87 moggers87 deleted the cleanup-old-cruft branch December 25, 2019 12:59
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.

3 participants