-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Update Travis CI matrix #1125
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
Update Travis CI matrix #1125
Conversation
Tests appear to be failing on Python 3.7 + MariaDB, and that appears to be unrelated to this pull request (which does not affect that configuration). |
Thanks. I have a strong preference for the Travis CI matrix without tox-travis. Compare https://travis-ci.org/jazzband/django-debug-toolbar/builds/462887366 to https://travis-ci.org/jazzband/django-debug-toolbar/builds/475893380 -- tox-travis bundles all Django versions into one build making it a bit more cumbersome to understand what's happening. That being said, since Django 1.11.17 officially supports Python 3.7 adding this combination would be worthwhile! |
@@ -1,6 +1,6 @@ | |||
[tox] | |||
envlist = | |||
py{27,34,35,36}-dj111 | |||
py{27,34,35,36,37}-dj111 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, please.
|
||
install: | ||
- pip install tox codecov | ||
- pip install tox tox-travis codecov |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please don't. tox-travis hides jobs for individual Django versions in the Travis CI build overview.
We both have differing ideas of how useful knowing the Django version that broke a build is on first glance. The alternative that I generally recommend for larger projects where combining parts of the matrix would cause absurdly long individual build times (think 20 mins vs 5 mins) is to set an environment variable such as You can see an example of a Travis matrix using that here: https://travis-ci.org/encode/django-rest-framework |
OK, I didn't know that it is possible to keep separate builds for Django versions even when using tox-travis. However, if you're doing this you're back to defining version combinations twice again, once in tox.ini and once in .travis.yml so I'm unclear how this is an improvement and/or a simplification when compared to what we're currently doing? |
Also regarding
In my* experience stuff breaks more often because of differences between Django versions than between Python versions (excluding 2.x vs. 3.x maybe, but not that much either anymore). I'm definitely not of the opinion that the Django version is less important.
|
FWIW, I too prefer avoiding tox-travis. But that is just my opinion and I understand this may come down to one's personal preference. |
I don't care much either way now that I learned that it is possible to keep individual builds for Django versions even with tox-travis. I still don't see how it simplifies the configuration (since you still have to specify the combinations twice), but won't stand in the way anymore. |
I misread what @jdufresne wrote (I didn't see "avoiding"). #1126 adds py37-dj111 to the build without adding tox-travis. Let's move forward with this. Thanks @kevin-brown for the idea! |
Fixes #1125. Requires pinning mysqlclient<1.4 because of mysqlclient's failure to interpolate named params into an SQL query (it converts strings to bytes and afterwards complains that it cannot find b"first_name", e.g. in test_raw_query_param_conversion: ... File "/home/travis/build/jazzband/django-debug-toolbar/.tox/mariadb/lib/python3.7/site-packages/MySQLdb/cursors.py", line 188, in execute query = query % args KeyError: b'first_name'
Uh oh!
There was an error while loading. Please reload this page.