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

Fix problem with multiple matches to shadowed module finder #1228

Merged
merged 1 commit into from
Mar 28, 2023

Conversation

corranwebster
Copy link
Contributor

This checks against the possibility of the ShadowedModuleFinder being used with different modules (eg. potentially other ETS modules)

This checks against the possibility of the ShadowedModuleFinder being used
with different modules (eg. potentially other ETS modules)
Copy link
Member

@mdickinson mdickinson left a comment

Choose a reason for hiding this comment

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

LGTM

Minor suggestion, orthogonal to this PR: would it make sense to remove the defaults from the implementation of ShadowedModuleFinder.__init__ and pass them explicitly at ShadowedModuleFinder creation time?

@corranwebster
Copy link
Contributor Author

Minor suggestion, orthogonal to this PR: would it make sense to remove the defaults from the implementation of ShadowedModuleFinder.__init__ and pass them explicitly at ShadowedModuleFinder creation time?

Possibly - I had them hard-coded because I didn't know whether or not I was going to copy or import in TraitsUI. I may still end up copying, so I will leave as-is for now until the TraitsUI analogue PR is merged.

@corranwebster corranwebster merged commit 38e60c6 into main Mar 28, 2023
@corranwebster corranwebster deleted the fix/shadow-finder-checks branch March 28, 2023 12:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants