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

DOCS: fix error in exec namespace note #119378

Merged
merged 2 commits into from
May 22, 2024

Conversation

ncoghlan
Copy link
Contributor

@ncoghlan ncoghlan commented May 22, 2024

When updating the new exec note added in gh-119235 as part of the PEP 667 general docs PR, I suggested a workaround that isn't valid.

Since it's far from the first time I've considered that workaround, and the fact it doesn't work has surprised me every time, amend the new note to explicitly state that dict merging is the only option.


📚 Documentation preview 📚: https://cpython-previews--119378.org.readthedocs.build/

When updating the new exec note added in pythongh-119235 as part of the
PEP 667 general docs PR, I suggested a workaround that isn't valid.

Since it's far from the first time I've considered that workaround,
and the fact it doesn't work has surprised me every time, amend the
new note to explicitly state that dict merging is the only option.
@ncoghlan ncoghlan added docs Documentation in the Doc dir skip issue skip news 3.13 bugs and security fixes 3.14 new features, bugs and security fixes needs backport to 3.13 bugs and security fixes labels May 22, 2024
@ncoghlan ncoghlan requested a review from gpshead May 22, 2024 05:00
@ncoghlan ncoghlan self-assigned this May 22, 2024
@ncoghlan
Copy link
Contributor Author

Only backporting to 3.13, since the incorrect workaround was never added to the 3.12 note.

Copy link
Member

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

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

I would
Just delete the incorrect workaround. It doesn’t seem very valuable to explicitly document something that doesn’t work, nor the cumbersome copying.

@ncoghlan
Copy link
Contributor Author

Good point, done (and set to merge once CI is happy).

@ncoghlan ncoghlan enabled auto-merge (squash) May 22, 2024 06:23
@ncoghlan ncoghlan merged commit 31d61a7 into python:main May 22, 2024
25 checks passed
@miss-islington-app
Copy link

Thanks @ncoghlan for the PR 🌮🎉.. I'm working now to backport this PR to: 3.13.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request May 22, 2024
When updating the new exec note added in pythongh-119235 as part of the
PEP 667 general docs PR, I suggested a workaround that isn't valid.

The first half of the note is still reasonable, so just omit the invalid text.
(cherry picked from commit 31d61a7)

Co-authored-by: Alyssa Coghlan <ncoghlan@gmail.com>
@bedevere-app
Copy link

bedevere-app bot commented May 22, 2024

GH-119380 is a backport of this pull request to the 3.13 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.13 bugs and security fixes label May 22, 2024
estyxx pushed a commit to estyxx/cpython that referenced this pull request Jul 17, 2024
When updating the new exec note added in pythongh-119235 as part of the
PEP 667 general docs PR, I suggested a workaround that isn't valid.

The first half of the note is still reasonable, so just omit the invalid text.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.13 bugs and security fixes 3.14 new features, bugs and security fixes docs Documentation in the Doc dir skip issue skip news
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants