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

Remove Python 2 support #116

Merged
merged 1 commit into from
May 29, 2019
Merged

Conversation

gergelypolonkai
Copy link
Collaborator

No description provided.

@gergelypolonkai gergelypolonkai force-pushed the remove-python2 branch 2 times, most recently from 0c51a50 to 95c4e4d Compare May 25, 2019 05:16
@coveralls
Copy link

coveralls commented May 25, 2019

Coverage Status

Coverage decreased (-1.6%) to 78.848% when pulling 0203c02 on gergelypolonkai:remove-python2 into b3d05d2 on sh4nks:master.

Copy link
Collaborator

@sh4nks sh4nks left a comment

Choose a reason for hiding this comment

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

Could you address the 2 comments I have made? Otherwise it looks good to me!

tox.ini Outdated Show resolved Hide resolved
flask_caching/_compat.py Outdated Show resolved Hide resolved
@gergelypolonkai
Copy link
Collaborator Author

I fixed what you asked for, and the tests still run fine. the coverage change looks strange, but looking it the numbers it didn’t actually change (the same amount of code lines are covered as before, we just lost some previously tested code.

Copy link
Collaborator

@sh4nks sh4nks left a comment

Choose a reason for hiding this comment

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

I made travis work with Python 3.7 here: https://github.com/flaskbb/flaskbb/blob/7b869949d9fb0c13bd0c12c79d556be7da2eba30/.travis.yml

But we can handle this in a separate PR :-)

@@ -13,7 +10,7 @@ def iteritems_wrapper(mappingorseq):

"""
if hasattr(mappingorseq, "items"):
return iteritems(mappingorseq)
return iter(mappingorseq.items())
Copy link
Collaborator

Choose a reason for hiding this comment

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

In Python 3, items() returns an iterator and not a list --> iter is not needed here

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, that was suspicious for me, i just couldn’t wrap my head around it early in the morning i guess.

@@ -97,7 +97,7 @@ def get_dict(self, *keys):
d = rv = self._client.get_multi(_keys)
if have_encoded_keys or self.key_prefix:
rv = {}
for key, value in iteritems(d):
for key, value in iter(d.items()):
Copy link
Collaborator

Choose a reason for hiding this comment

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

iter(d.items()) --> d.items()

@gergelypolonkai
Copy link
Collaborator Author

There, everything is fixed. Take a look and merge when you see fit.

@sh4nks sh4nks merged commit 6976799 into pallets-eco:master May 29, 2019
@sh4nks
Copy link
Collaborator

sh4nks commented May 29, 2019

Everything looks good now! Thanks!

@gergelypolonkai gergelypolonkai deleted the remove-python2 branch May 29, 2019 09:23
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 21, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants