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

gh-112414: Add additional unit tests for calling repr() on a namespace package #112475

Merged
merged 5 commits into from
Nov 28, 2023

Conversation

AlexWaygood
Copy link
Member

@AlexWaygood AlexWaygood commented Nov 27, 2023

Addressing @ericsnowcurrently's post-merge review at #112425 (comment). I've verified locally that the second test here fails if you build CPython with e954ac7 (the commit prior to 0622839).

@ericsnowcurrently, would you also like me to remove the test I added in 0622839, or is that okay to stay? :)

@ericsnowcurrently
Copy link
Member

@ericsnowcurrently, would you also like me to remove the test I added in 0622839, or is that okay to stay? :)

Removing that test makes sense, since the test you add here covers that bit and the former is a bit out of place.

Copy link
Member

@ericsnowcurrently ericsnowcurrently left a comment

Choose a reason for hiding this comment

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

mostly LGTM

@bedevere-app
Copy link

bedevere-app bot commented Nov 27, 2023

When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.

Copy link
Member

@ericsnowcurrently ericsnowcurrently left a comment

Choose a reason for hiding this comment

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

Also, s/"/'/g

AlexWaygood and others added 3 commits November 27, 2023 23:49
Co-authored-by: Eric Snow <ericsnowcurrently@gmail.com>
…rily

Co-authored-by: Eric Snow <ericsnowcurrently@gmail.com>
@AlexWaygood
Copy link
Member Author

Thanks! I have made the requested changes; please review again

Copy link
Member

@ericsnowcurrently ericsnowcurrently left a comment

Choose a reason for hiding this comment

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

LGTM

@AlexWaygood AlexWaygood merged commit cf20540 into python:main Nov 28, 2023
@AlexWaygood AlexWaygood deleted the more-module-repr-tests branch November 28, 2023 00:10
@miss-islington-app
Copy link

Thanks @AlexWaygood for the PR 🌮🎉.. I'm working now to backport this PR to: 3.12.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Nov 28, 2023
…namespace package (pythonGH-112475)

(cherry picked from commit cf20540)

Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
Co-authored-by: Eric Snow <ericsnowcurrently@gmail.com>
@bedevere-app
Copy link

bedevere-app bot commented Nov 28, 2023

GH-112480 is a backport of this pull request to the 3.12 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.12 bug and security fixes label Nov 28, 2023
@ericsnowcurrently
Copy link
Member

Thanks for working on this, @AlexWaygood!

@AlexWaygood
Copy link
Member Author

You're very welcome! :D

AlexWaygood added a commit that referenced this pull request Nov 28, 2023
… namespace package (GH-112475) (#112480)

gh-112414: Add additional unit tests for calling `repr()` on a namespace package (GH-112475)
(cherry picked from commit cf20540)

Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
Co-authored-by: Eric Snow <ericsnowcurrently@gmail.com>
aisk pushed a commit to aisk/cpython that referenced this pull request Feb 11, 2024
…namespace package (python#112475)

Co-authored-by: Eric Snow <ericsnowcurrently@gmail.com>
Glyphack pushed a commit to Glyphack/cpython that referenced this pull request Sep 2, 2024
…namespace package (python#112475)

Co-authored-by: Eric Snow <ericsnowcurrently@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip news tests Tests in the Lib/test dir topic-importlib
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants