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

Fixes for python3.10 #1824

Merged
merged 1 commit into from Oct 27, 2019

Conversation

@asottile
Copy link
Contributor

asottile commented Aug 18, 2019

Summary of changes

found using asottile/python3.10

Pull Request Checklist

  • Changes have tests -- 17 tests failed prior to this change
  • News fragment added in changelog.d. See documentation for details
@asottile asottile force-pushed the asottile:python310 branch from cceb810 to b56730f Aug 18, 2019
@asottile

This comment has been minimized.

Copy link
Contributor Author

asottile commented Aug 30, 2019

@pganssle would appreciate a review <3

@pganssle

This comment has been minimized.

Copy link
Member

pganssle commented Aug 30, 2019

I think it might be good to pull this from a centralized place rather than repeating the same formatting idiom in a bunch of places.

@asottile

This comment has been minimized.

Copy link
Contributor Author

asottile commented Aug 30, 2019

sounds good -- do you have a recommended place to put this little helper "constant"?

@asottile

This comment has been minimized.

Copy link
Contributor Author

asottile commented Sep 30, 2019

@pganssle ping! should I put these in some sort of _compat module?

@pganssle

This comment has been minimized.

Copy link
Member

pganssle commented Oct 3, 2019

@asottile I dunno if _compat is right, it's more like _environment or _platform or something.

The one in setup.py can stay as it is, though.

@jaraco Do you have any suggestions, code-organization-wise?

@jaraco

This comment has been minimized.

Copy link
Member

jaraco commented Oct 27, 2019

Honestly, I struggle with a good place to put this logic, because the usage appears in

  • setuptools/* (4)
  • pkg_resources/* (including doctests) (3)
  • setup.py (1)

There's just no good place to this indication. Here's what I suggest - let's wait for this idiom (or the resulting value) to appear in packaging (on which both pkg_resources and setuptools both depend) and then rely on it from there (except maybe for setup.py).

And in the meantime, let's accept it as proposed.

@jaraco jaraco merged commit e3068ee into pypa:master Oct 27, 2019
5 of 6 checks passed
5 of 6 checks passed
codecov/patch 66.66% of diff hit (target 84.73%)
Details
Summary 1 potential rule
Details
codecov/project 84.73% remains the same compared to 2cd2fdc
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
deploy/netlify Deploy preview ready!
Details
@asottile asottile deleted the asottile:python310 branch Oct 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.