-
-
Notifications
You must be signed in to change notification settings - Fork 70
Conda setup for CI #181
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
Conda setup for CI #181
Conversation
nicoddemus
left a comment
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.
@rth thanks a ton, this is great work.
I made a few comments, please take a look.
.travis.yml
Outdated
| - sudo $PYTHON -m pip install -U pytest tox coveralls | ||
| - | | ||
| if [[ ${PYTEST_QT_API} == 'pyside' ]]; then | ||
| pyqt_package="pyside==1.2.4" |
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.
What do you think about creating a PYQT_PACKAGE environment variable in the env section along with the others? There will be a small repetition, but I think that's OK.
.travis.yml
Outdated
| - sudo $PYTHON -m pip install -U pytest tox coveralls | ||
| - | | ||
| if [[ ${PYTEST_QT_API} == 'pyside' ]]; then | ||
| pyqt_package="pyside==1.2.4" |
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.
I think we can use fuzzy versions like pyside=1.*, pyqt=4.* and pyqt=5.*.
|
Also please make sure to add yourself as a contributor in the |
|
@nicoddemus Thanks a lot for the review! I addressed your comments and added the Appveyor setup (largely inspired by the one in qtpy). Also refactored a bit travis setup to be more consistent. Tests now pass. The only point is that in Travis there is a warning/error at the very end, but I guess since tests pass that's not relevant... |
|
Also, there is no linting in Appveyor, but since it's already done in Travis that should, hopefully, be sufficient... |
nicoddemus
left a comment
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.
Awesome!
I agree that we don't need to also run linting on AppVeyor, that's fine.
I commented a minor detail if you can address it, but if you are out of time we can merge as it is. 👍
appveyor.yml
Outdated
| CONDA_DEPENDENCIES: "pytest tox coveralls pyside=1.*" | ||
| - PYTHON_VERSION: "3.4" | ||
| PYTEST_QT_API: "pyqt5" | ||
| CONDA_DEPENDENCIES: "pytest tox coveralls pyqt=5.*" |
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.
We are not using tox and coveralls here so I guess we can remove them.
|
Thanks, you are right - I removed tox and coveralls from the Appveyor setup.. |
|
Awesome, thank you, really appreciate it! 👍 |
Following discussion in #180 this PR proposes a Travis CI setup using conda. Please let me know if I missed anything.
Edit: with the appveyor.yml addition this fixes #180