Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

bpo-21761: Clarify __file__/__cached__ in import reference#31565

Merged
miss-islington merged 4 commits into
python:mainpython/cpython:mainfrom
slateny:s/importslateny/cpython:s/importCopy head branch name to clipboard
Apr 26, 2022
Merged

bpo-21761: Clarify __file__/__cached__ in import reference#31565
miss-islington merged 4 commits into
python:mainpython/cpython:mainfrom
slateny:s/importslateny/cpython:s/importCopy head branch name to clipboard

Conversation

@slateny

@slateny slateny commented Feb 25, 2022 •
edited by miss-islington
Loading

Copy link
Copy Markdown
Contributor

https://bugs.python.org/issue21761

Automerge-Triggered-By: GH:brettcannon

@bedevere-bot bedevere-bot added the docs Documentation in the Doc dir label Feb 25, 2022
Vidhyavinu
Vidhyavinu reviewed Feb 28, 2022
View reviewed changes

@Vidhyavinu Vidhyavinu left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I feel "-the spec instructs the loader" can be started as a new sentence. Rest looks good.
@ericsnowcurrently : are you ok with this?

@slateny

slateny commented Mar 1, 2022

Copy link
Copy Markdown
Contributor Author

I feel "-the spec instructs the loader" can be started as a new sentence. Rest looks good. @ericsnowcurrently : are you ok with this?

Separating the phrase into a new sentence felt a bit abrupt so I connected it to the previous sentence instead, but that's a preference thing and I'm open to changing it back if the others concur.

@slateny slateny changed the title bpo-21761: Clarified __file__/__cached__ in import reference bpo-21761: Clarify __file__/__cached__ in import reference Mar 4, 2022
ericsnowcurrently
ericsnowcurrently requested changes Mar 4, 2022
View reviewed changes

@ericsnowcurrently ericsnowcurrently left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for working on this, @slateny! I've made some recommendations for you to consider.

Also, in bpo-21761 I said, "The entire section should have a clear note that loaders are not meant to rely a module's import-related attributes." This PR should include the addition of such a note.

Comment thread Doc/reference/import.rst Outdated
Comment thread Doc/reference/import.rst Outdated
@bedevere-bot

bedevere-bot commented Mar 4, 2022

Copy link
Copy Markdown

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

slateny and others added 2 commits March 5, 2022 01:41
@slateny @ericsnowcurrently
Update Doc/reference/import.rst
c66385d 
Co-authored-by: Eric Snow <ericsnowcurrently@gmail.com>
@slateny
Update import.rst
80e1f3b
@slateny

slateny commented Mar 5, 2022

Copy link
Copy Markdown
Contributor Author

I have made the requested changes; please review again

@bedevere-bot

bedevere-bot commented Mar 5, 2022

Copy link
Copy Markdown

Thanks for making the requested changes!

@ericsnowcurrently: please review the changes made to this pull request.

@brettcannon brettcannon reopened this Apr 26, 2022
@miss-islington

miss-islington commented Apr 26, 2022

Copy link
Copy Markdown
Contributor

@slateny: Status check is done, and it's a success ✅ .

@miss-islington miss-islington merged commit b87f7f2 into python:main Apr 26, 2022
@brettcannon

brettcannon commented Apr 26, 2022

Copy link
Copy Markdown
Member

Thanks!

@slateny slateny deleted the s/import branch May 15, 2022 02:28
@ericsnowcurrently ericsnowcurrently mentioned this pull request Apr 10, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@brettcannon brettcannon brettcannon approved these changes

@ericsnowcurrently ericsnowcurrently Awaiting requested review from ericsnowcurrently ericsnowcurrently is a code owner

+1 more reviewer

@Vidhyavinu Vidhyavinu Vidhyavinu left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

docs Documentation in the Doc dir skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@slateny @bedevere-bot @miss-islington @brettcannon @ericsnowcurrently @Vidhyavinu @the-knights-who-say-ni @AlexWaygood
Morty Proxy This is a proxified and sanitized view of the page, visit original site.