-
-
Notifications
You must be signed in to change notification settings - Fork 441
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
Provide an API for passing plugins to the Coverage constructor #1919
Conversation
Hmm, what's the correct idiom for documenting |
I'll have to figure out what to do about the docs. It might be that we type it as You made the passed list of plugins override the ones found in the config. Is there a reason for that? I would have thought it would be additive. |
Sure, I can switch to bare As for why override, I went with the first sentence in the If you have a preference for them being additive instead, happy to switch, I don't feel strongly. |
@nedbat would you prefer I switch to adding, instead of replacing? |
You make a good case: "read Ned his own docs to show that I was right" :D |
:D Thanks for the review and merge! |
mod = sys.modules[module] | ||
if plugin_override is not None: | ||
for override in plugin_override: | ||
override(plugins, {}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Of course, after merging, thinking about documenting this, I have a hard time explaining why config options are passed to this function but they are always empty. I'm going to change the signature of the plugin function to leave it out.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok! I went for consistency with the signature of plugin inits loaded from the config, but that did produce this slightly silly result.
Also simplify the callable signature for plugin overrides, since the second config argument would always be empty.
This is now released as part of coverage 7.7.0. |
❤️ thanks! |
No description provided.