-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Drop support for EOL Python 3.3 #1342
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
Conversation
@@ -1,17 +1,3 @@ | |||
__all__ = ['get_config_vars', 'get_path'] |
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.
Why drop __all__
here?
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.
get_config_vars
and get_path
have been removed from py31compat.py because Python 2.7 and 3.4+ can directly from sysconfig import get_config_vars, get_path
.
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.
Shouldn't we change this to __all__ = []
then?
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 don't think it's necessary, if nothing is exposed.
The others don't. For example: https://github.com/pypa/setuptools/blob/master/setuptools/py36compat.py
But happy to add it if you prefer!
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 it's probably better to be explicit, since if __all__
is not specified, from py31compat.py import *
will import everything, but if __all__
is set to []
it will import nothing.
I think it also sends a clearer message that this module provides no public members.
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.
Updated!
I think the diff coverage that is failing is not a problem. The changes seem sane in the uncovered portions, and it's just that it's changing things that aren't tested in the first place. Per #1339, this should be deferred until the next breaking release. |
pytest-flake8 1.0.1 has a hard dependency on pytest>=3.5 [1], which does not support python3.3. Pin it until python3.3 is removed [3]. [1] tholo/pytest-flake8@25bbd3b [2] https://docs.pytest.org/en/latest/changelog.html#pytest-3-3-0-2017-11-23 [3] pypa#1342
pytest-flake8 1.0.1 has a hard dependency on pytest>=3.5 [1], which does not support python3.3 [2]. Pin it until python3.3 is removed [3]. [1] tholo/pytest-flake8@25bbd3b [2] https://docs.pytest.org/en/latest/changelog.html#pytest-3-3-0-2017-11-23 [3] pypa#1342
pytest-flake8 1.0.1 has a hard dependency on pytest>=3.5 [1], which does not support python3.3 [2]. Avoid it for python3.3 and below until python3.3 is removed [3]. [1] tholo/pytest-flake8@25bbd3b [2] https://docs.pytest.org/en/latest/changelog.html#pytest-3-3-0-2017-11-23 [3] pypa#1342
pytest-flake8 1.0.1 has a hard dependency on pytest>=3.5 [1], which does not support python3.3 [2]. Avoid it for python3.3 and below until python3.3 is removed [3]. Additionally, ensure we have a recent enough pip in travis ci to handle multiple requirements entries. [1] tholo/pytest-flake8@25bbd3b [2] https://docs.pytest.org/en/latest/changelog.html#pytest-3-3-0-2017-11-23 [3] pypa#1342
pytest-flake8 1.0.1 has a hard dependency on pytest>=3.5 [1], which does not support python3.3 [2]. Avoid it for python3.3 and below until python3.3 is removed [3]. Additionally, ensure we have a recent enough pip in travis ci to handle multiple requirements entries. [1] tholo/pytest-flake8@25bbd3b [2] https://docs.pytest.org/en/latest/changelog.html#pytest-3-3-0-2017-11-23 [3] pypa#1342
It looks like the |
@jaraco @benoit-pierre Now that Do we have any breaking changes already in master? If not, should we cut a minor release now and then land this PR? |
It's been a few days since the last release, so I think we can merge this now. CC: @jmbowman |
As suggested in #1339, as it's EOL and Tox no longer supports it.
Also remove redundant code and update other code.