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

unpin requirements and make testing use tox #325

Closed

Conversation

yiwensong
Copy link

  • marked a bunch of tests as broken (because they were broken for me)
  • switch to using coverage instead of pytest-cov because pytest-cov does not support py36
  • unpin requirements to make installation less collision with other packages
  • add python 3.6

Copy link
Contributor

@alimcmaster1 alimcmaster1 left a comment

Choose a reason for hiding this comment

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

Hey, thanks @yiwensong for the contribution, generally looks great to me. Ive also started some very similar changes. Will submit a PR too and we can compare and consolidate

@@ -3,16 +3,18 @@
from setuptools import setup, find_packages

install_requires = [
'dateutils',
'sortedcontainers>=1.5.9',
Copy link
Contributor

Choose a reason for hiding this comment

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

these are test requirements that we don't want in the setup.py

pymongo>=3.5.1
pytest
requests>=2.13.0
six>=1.10.0
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can get rid of this file and just have our requirements in setup.py. No point maintaining versions in two places

Copy link
Author

Choose a reason for hiding this comment

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

I think this file is used in the Dockerfile? I haven't looked into what the docker does, but I kept it in case it was important.

@alimcmaster1
Copy link
Contributor

For reference please see #326 where I have fixed up our CI. If you have any improvements let me know :)

@yiwensong
Copy link
Author

@alimcmaster1 I'll drop my PR because it looks like we're doing very similar things. I'll make a new one on top of your branch that unpins the requirements.

@yiwensong yiwensong closed this Aug 27, 2018
@alimcmaster1
Copy link
Contributor

alimcmaster1 commented Aug 27, 2018 via email

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.

None yet

2 participants