[proxy] github.com — ← back | site home | direct (HTTPS) ↗ | proxy home | ◑ dark◐ light
/ cpython Public

Conversation

Copy link
Contributor

ambv commented Jun 9, 2020 •
edited
Loading

This reverts commit 9cf1be4 due to

https://bugs.python.org/issue40924

The change being reverted broke Certifi and caused root certificates to be unavailable to urllib.

Copy link
Member

vstinner commented Jun 9, 2020

This PR looks larger than just a revert. It also upgrade to the beta2. Was it done on purpose?

Copy link
Contributor Author

ambv commented Jun 9, 2020

No, that's a failed rebase. Fixing right now.

ambv force-pushed the revert_importlib_resources branch from 1797ad9 to c791333 Compare June 9, 2020 13:03
ambv added the skip news label Jun 9, 2020
Copy link
Member

vstinner commented Jun 9, 2020 •
edited by bedevere-bot
Loading

Please add a NEWS entry to mention the revert.

9cf1be4 added Misc/NEWS.d/next/Library/2020-06-02-02-16-02.[bpo-39791](https://bugs.python.org/issue39791).StCJlA.rst which was merged into 3.9.0b2.rst:

Built-in loaders (SourceFileLoader and ZipImporter) now supply ``TraversableResources`` implementations for ``ResourceReader``, and the fallback function has been removed.

vstinner approved these changes Jun 9, 2020
View reviewed changes
Copy link
Member

vstinner left a comment

Choose a reason for hiding this comment

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

LGTM if you add a NEWS entry.

Copy link
Member

vstinner commented Jun 9, 2020

IMHO it's better to document the revert than modifying Misc/NEWS.d/3.9.0b2.rst.

ambv removed the skip news label Jun 9, 2020
ned-deily approved these changes Jun 9, 2020
View reviewed changes
Copy link
Member

ned-deily left a comment

Choose a reason for hiding this comment

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

I did a complete macOS installer build and test install with the PR. The test suite ran without errors and I verified that certifi.where() now returns the correct result. With the addition of a news entry, LGTM.

ned-deily reviewed Jun 9, 2020
View reviewed changes
Copy link
Member

ned-deily left a comment

Choose a reason for hiding this comment

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

Perhaps the entry should specifically mention "introduced in 3.9.0b2"?

Copy link
Contributor Author

ambv commented Jun 9, 2020

Good idea, Ned.

ambv merged commit ce5e6f0 into python:3.9 Jun 9, 2020
Copy link

bedevere-bot commented Jun 9, 2020

@ambv: Please replace # with GH- in the commit message next time. Thanks!

ambv deleted the revert_importlib_resources branch June 9, 2020 17:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

vstinner vstinner approved these changes

ned-deily ned-deily approved these changes

brettcannon Awaiting requested review from brettcannon brettcannon is a code owner

encukou Awaiting requested review from encukou encukou is a code owner

ericsnowcurrently Awaiting requested review from ericsnowcurrently ericsnowcurrently is a code owner

ncoghlan Awaiting requested review from ncoghlan ncoghlan is a code owner

rhettinger Awaiting requested review from rhettinger

skrah Awaiting requested review from skrah

warsaw Awaiting requested review from warsaw warsaw is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants