Skip to content

Conversation

@graingert
Copy link
Member

@graingert graingert commented Dec 23, 2020

see also #104 (comment)


This change is Reviewable

@skarzi
Copy link
Contributor

skarzi commented Dec 23, 2020

@graingert could you merge master? or (if it's easier) run pyupgrade on master again?

@graingert graingert force-pushed the drop-support-for-eol-pythons branch from bdd2b89 to 330852b Compare December 23, 2020 14:42
@skarzi
Copy link
Contributor

skarzi commented Dec 23, 2020

Let's apply black on setup.py file - https://travis-ci.org/github/pytest-dev/pytest-factoryboy/jobs/751201487#L697

@graingert graingert force-pushed the drop-support-for-eol-pythons branch from 330852b to 1c664cb Compare December 23, 2020 15:11
setup.cfg Outdated
Topic :: Software Development :: Libraries
Topic :: Utilities
Programming Language :: Python :: 3
Programming Language :: Python :: 3 :: Only
Copy link
Contributor

Choose a reason for hiding this comment

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

I wouldn't use this classifier, as it implies that the project would not be compatible with a hypothetical python 4

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we have to match pytest's config?

Copy link
Member

Choose a reason for hiding this comment

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

As far as we know, it's not compatible with Python 4...

Copy link
Contributor

Choose a reason for hiding this comment

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

I disagree with this, if we follow this logic then we should also add python_requires<4, and we don't do that.
Python 4 will probably not introduce incompatible changes like python 3 did.

Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove this classifier.

@graingert graingert requested review from hugovk and youtux December 23, 2020 15:35
setup.cfg Outdated
Topic :: Software Development :: Libraries
Topic :: Utilities
Programming Language :: Python :: 3
Programming Language :: Python :: 3 :: Only
Copy link
Contributor

Choose a reason for hiding this comment

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

I disagree with this, if we follow this logic then we should also add python_requires<4, and we don't do that.
Python 4 will probably not introduce incompatible changes like python 3 did.

@graingert graingert changed the title apply black to docs/conf.py drop support for EOL python 2 Dec 23, 2020
Copy link
Member

@hugovk hugovk left a comment

Choose a reason for hiding this comment

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

Looks good! Presence or absence of the 3 :: Only classifier isn't a big deal, would be good to add it 3.9 as well.

@graingert
Copy link
Member Author

@hugovk 3.9 classifier is already present

@graingert graingert requested a review from youtux December 23, 2020 17:03
@hugovk
Copy link
Member

hugovk commented Dec 23, 2020

👍

And add 3.9 to CI?

Comment on lines 2 to 3
from .fixture import LazyFixture
from .fixture import register
Copy link
Contributor

Choose a reason for hiding this comment

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

why the previous approach was wrong?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is using reorder_python_imports, the same as pytest

Copy link
Contributor

Choose a reason for hiding this comment

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

please revert this. Let's have a PR with one objective, if we want to introduce a new tool to this project it should be a separate PR where we can discuss that.

Copy link
Member Author

Choose a reason for hiding this comment

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

@youtux I've reverted the change to use reorder-python-imports, and will make it an a followup

@skarzi
Copy link
Contributor

skarzi commented Dec 23, 2020

And add 3.9 to CI?

3.9 is already tested by CI pipeline, however, 3.10 is missing in .travis.yml and tox.ini

@graingert graingert requested a review from youtux December 26, 2020 16:07
setup.cfg Outdated
Topic :: Software Development :: Libraries
Topic :: Utilities
Programming Language :: Python :: 3
Programming Language :: Python :: 3 :: Only
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove this classifier.

@youtux youtux merged commit 72eb55b into pytest-dev:master Dec 30, 2020
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.

4 participants