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 support for legacy Python 2.7 #230

Merged
merged 9 commits into from
Sep 9, 2019
Merged

Conversation

hugovk
Copy link
Member

@hugovk hugovk commented Aug 15, 2019

Fixes #228.

Copy link
Contributor

@BeyondEvil BeyondEvil left a comment

Choose a reason for hiding this comment

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

Awesome job! Thanks for this!

I'm not sure how to handle the merge however. It is after all a couple of months left before 2.7 is EOL.
@davehunt @rochacbruno

pytest_html/plugin.py Outdated Show resolved Hide resolved
pytest_html/plugin.py Outdated Show resolved Hide resolved
pytest_html/plugin.py Outdated Show resolved Hide resolved
pytest_html/plugin.py Outdated Show resolved Hide resolved
pytest_html/plugin.py Outdated Show resolved Hide resolved
pytest_html/plugin.py Outdated Show resolved Hide resolved
tox.ini Outdated Show resolved Hide resolved
tox.ini Outdated Show resolved Hide resolved
@hugovk
Copy link
Member Author

hugovk commented Aug 18, 2019

Thanks for the review, updated!


I'm not sure how to handle the merge however. It is after all a couple of months left before 2.7 is EOL.

To compare with pytest itself:

Copy link
Contributor

@BeyondEvil BeyondEvil left a comment

Choose a reason for hiding this comment

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

Sorry that I didn't catch all those the previous round. 😞

testing/test_pytest_html.py Outdated Show resolved Hide resolved
testing/test_pytest_html.py Show resolved Hide resolved
testing/test_pytest_html.py Outdated Show resolved Hide resolved
testing/test_pytest_html.py Outdated Show resolved Hide resolved
testing/test_pytest_html.py Outdated Show resolved Hide resolved
testing/test_pytest_html.py Outdated Show resolved Hide resolved
testing/test_pytest_html.py Outdated Show resolved Hide resolved
testing/test_pytest_html.py Outdated Show resolved Hide resolved
testing/test_pytest_html.py Outdated Show resolved Hide resolved
testing/test_pytest_html.py Outdated Show resolved Hide resolved
@BeyondEvil
Copy link
Contributor

Good job on the changes! Discovered some more, sorry about that.

Thanks for the review, updated!

I'm not sure how to handle the merge however. It is after all a couple of months left before 2.7 is EOL.

To compare with pytest itself:

* Pytest 5.x is the master branch, and no longer supports Python 2.7.

* The pytest core team will make bug fixes to pytest 4.6 until January 2020, and will accept and release community patches until mid-2020:

* https://docs.pytest.org/en/latest/py27-py34-deprecation.html

Not a bad idea. I guess we can have a twodotseven-branch for the same purpose.

But I want someone else to chime in before I make it so. @davehunt @rochacbruno

pytest_html/plugin.py Outdated Show resolved Hide resolved
@RibeiroAna
Copy link
Member

@BeyondEvil my opinion is that we should launch Pytest-HTML 2.0 with this patch and people who still need to use Python 2.7 during the remaining months until its dead should use Pytest-HTML 1.x

@BeyondEvil
Copy link
Contributor

@BeyondEvil my opinion is that we should launch Pytest-HTML 2.0 with this patch and people who still need to use Python 2.7 during the remaining months until its dead should use Pytest-HTML 1.x

I agree. I'm just not sure how to handle the practical aspects.

@RibeiroAna
Copy link
Member

@BeyondEvil I'm not sure which practical aspects you mean! Maybe launch Pytest 1.23.0 that includes a deprecation alert on the HTML file? (I don't think it is worthy)

@BeyondEvil
Copy link
Contributor

@BeyondEvil I'm not sure which practical aspects you mean! Maybe launch Pytest 1.23.0 that includes a deprecation alert on the HTML file? (I don't think it is worthy)

Well, if master is going to be 2.x, we'll need a 1.x-"master"-branch, and I'm not sure if either of us can create that or if only @davehunt can do it.

And I agree, not worth releasing a 1.x version with just a deprecation warning. pytest itself already does that.

Copy link
Contributor

@BeyondEvil BeyondEvil left a comment

Choose a reason for hiding this comment

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

Probably a good idea to not allow a pytest version that supports 2.7

setup.py Outdated Show resolved Hide resolved
@hugovk
Copy link
Member Author

hugovk commented Aug 19, 2019

Thanks for the new reviews! I'm travelling right now and can update these later this week. Or feel free to update this branch.

The python_requires=">=3.6", here helps with some of the practical aspects. It means people (with modern pip) who install the package will only get this version if their Python version matches. Otherwise pip'll check each older one until it finds one that matches (or has no python_requires). So 2.7 users will get the last one to support 2.7.

https://python3statement.org/practicalities/ has some other suggestions, but I think python_requires is the most important.

@BeyondEvil
Copy link
Contributor

@hugovk No worries, there's no stress! We're not fixing anything that is broken.

Safe travels!

@hugovk
Copy link
Member Author

hugovk commented Aug 23, 2019

Updated!

@davehunt
Copy link
Collaborator

I'd say let's release this as pytest-html 2.0 and create a 1.22-maintenance branch, following the same approach as pytest, and only uplifting bugfixes to that branch and releasing 1.22.x as needed. I don't expect there will be many (if any) patches that will require a 1.22.x release though, and we should strongly encourage users to upgrade to Python 3.

@BeyondEvil BeyondEvil merged commit e6e5ef2 into pytest-dev:master Sep 9, 2019
@hugovk hugovk deleted the rm-2 branch September 9, 2019 15:16
@ssbarnea ssbarnea added the major Marks an important and likely breaking change. label Aug 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
major Marks an important and likely breaking change.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Deprecate compability with python 2.7
6 participants