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

Drop stale Python versions, test only on new ones #12

Open
wants to merge 13 commits into
base: master
Choose a base branch
from

Conversation

Toshakins
Copy link

@Toshakins Toshakins commented Jan 27, 2021

Hey, it's time to upgrade! Very minor code changes, just want to be sure everything runs on Py3.6+. Please let me know if any further work needed, I'm willing to commit my efforts. Thanks!

@Toshakins Toshakins marked this pull request as ready for review January 27, 2021 14:43
@Toshakins Toshakins changed the title - Drop stale Python versions, test only on new ones Drop stale Python versions, test only on new ones Jan 27, 2021
@thomasw
Copy link
Owner

thomasw commented Jan 28, 2021

@Toshakins, thanks for the contribution. Can you make a pass at updating all of the dev dependencies to the latest versions? You'll need to power through that to get the test suite to run and then we can go from there.

@thomasw
Copy link
Owner

thomasw commented Jan 28, 2021

I spun up a branch for you that has these changes: https://github.com/thomasw/querylist/compare/update-dev-dependencies?expand=1

thomasw and others added 5 commits January 27, 2021 23:55
This removes selective requirement installation for python <3 as it is no longer needed.
Set the package's long description to the contents of the readme.
@Toshakins
Copy link
Author

Sorry for those broken tests. Travis was queuing the build for a while, and I've lost patience. Merged the branch you mentioned, tests pass on py3.6, waiting for Travis.

@coveralls
Copy link

coveralls commented Jan 28, 2021

Coverage Status

Coverage remained the same at 98.473% when pulling 8abe26f on Toshakins:master into 4304023 on thomasw:master.

@Toshakins
Copy link
Author

Tests fail for pypy interpreter. Would it be cheating to make this library available for pypy3 only?

@thomasw
Copy link
Owner

thomasw commented Jan 28, 2021

Tests fail for pypy interpreter. Would it be cheating to make this library available for pypy3 only?

Not cheating at all and totally inline with the other changes. Thanks.

@@ -1,4 +1 @@
try:
Copy link
Owner

Choose a reason for hiding this comment

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

We can eliminate this file now and directly import TestCase in the test files.

Copy link
Author

Choose a reason for hiding this comment

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

As a consequence, I re-ordered imports in tests folder. Not sure if you have a strong opinion on imports, but anyway I'm okay to change them if needed.

setup.py Outdated
'Programming Language :: Python :: 3.7',
'Programming Language :: Python :: 3.8',
'Programming Language :: Python :: 3.9',
'Programming Language :: Python :: Implementation :: PyPy'
Copy link
Owner

Choose a reason for hiding this comment

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

Ugh, let's also add

Programming Language :: Python :: Implementation :: CPython

@@ -26,7 +26,7 @@ Querylist can be installed like any other python package:

> pip install querylist

Querylist is tested against Python 2.6, 2.7, 3.3, 3.4, and pypy.
Querylist is tested against Python 3.6+ and pypy.
Copy link
Owner

Choose a reason for hiding this comment

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

There might be duplicate content in docs/ that needs updating.

Copy link
Author

Choose a reason for hiding this comment

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

Thanks, updated docs too. Also, I allowed myself to update CHANGELOG.

@thomasw
Copy link
Owner

thomasw commented Jan 28, 2021

@Toshakins I'm assuming you're making updates here to ship a python3 upgrade for something Yola related?

What's your internal timeline looking like? Once we get this merged, I may try to get a little more cleanup in before shipping a new release.

I encourage you to rigorously verify that whatever depends on this in a python3 environment continues to work as expected. I haven't used this library myself in a while and I'm kinda just moving forward here assuming that the test suite I remember nothing about is sufficient to verify that things work as expected.

Also, please tell V that I said hello!

@Toshakins
Copy link
Author

I'm assuming you're making updates here to ship a python3 upgrade for something Yola related?

Yes! You're right. We are migrating to Python 3.6, actually.

What's your internal timeline looking like?

We tested the latest available version, and it worked, so we decided to ship with 0.4.0. But next generations of Yola people might be extremely confused by that decision. The project to upgrade Querylist does not have any schedule, really. It is my initiative, and I was hoping the whole upgrade won't take much effort. On the other hand, you didn't respond to the pull request we would consider forking and upgrading :) Feel free to do any additional cleanup, there are no time frames for that.

Also, please tell V that I said hello!

Nice! He hellos(is it a word?) you back!

@Toshakins
Copy link
Author

I've applied fixes for the comments above.

@thomasw
Copy link
Owner

thomasw commented Feb 11, 2021

I'm hoping to get to this over the weekend. I have a branch where I've rewritten some of your history and bundled some related cleanup. Please @mention me if you make additional changes here.

@Toshakins
Copy link
Author

Hello again! Caring about repositories could be hard. Huge thanks that you were able to take a look into my PR. Is it possible to publish a new version? It's ok if you have little time for that. I wanted only to follow up and do not want to push the release.

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