Safely ignore modules named cz_ that are not plugins
#480
Conversation
When a package starting with a plugin name is found, but it is not a plugin, it becomes safely ignored.
…not-plugins fix: Ignore packages that are not plugins
|
Hi, This is a terrible PR made at the wrong time of the day from the wrong operating system. I'll fix this and update the PR in the next days. |
Codecov Report
@@ Coverage Diff @@
## master #480 +/- ##
==========================================
+ Coverage 98.04% 98.05% +0.01%
==========================================
Files 39 39
Lines 1431 1441 +10
==========================================
+ Hits 1403 1413 +10
Misses 28 28
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
|
This pull request is ready to review. Please let me know if you require some additional changes. |
|
Looks good to me, @Lee-W ? |
| try: | ||
| if name.startswith("cz_"): | ||
| plugins[name] = importlib.import_module(name).discover_this # type: ignore | ||
| except AttributeError as e: |
There was a problem hiding this comment.
Overall, I like the idea. Most of the part looks good to me
@woile do you think we should go with the exit code method in this case as well?
There was a problem hiding this comment.
What do you mean? like a code to easily identify where this warning is coming from?
I'm not sure it's needed, the warning should point to the line in the code AFAIK 🤔
There was a problem hiding this comment.
like what we define here so that user can decide whether to suppress the warning. but I'm ok with this PR now. we could create another one if we think that's necessary. it won't break anything if we do that in the future
There was a problem hiding this comment.
I just approved it. If you agree with not handling this at this moment, I think we're good to merge it
| try: | ||
| if name.startswith("cz_"): | ||
| plugins[name] = importlib.import_module(name).discover_this # type: ignore | ||
| except AttributeError as e: |
There was a problem hiding this comment.
like what we define here so that user can decide whether to suppress the warning. but I'm ok with this PR now. we could create another one if we think that's necessary. it won't break anything if we do that in the future
When a package starting with a plugin name is found, but it is not a plugin, it becomes safely ignored.
Description
When a packages has a name that matches a plugin, but it is not a plugin, it is safely ignored and reported to the user.
Background: The company where I work, and where I use
commitizenfor releasing package versions, is called CZ. And because we are not very imaginative, we use to call the packages that we develop for internal use something likecz_.This collides with the way that
commitizendiscover plugins, and the discovery process raises an exception when a package starts with the namecz_is found but is not implemented as a plugin.This MR proposes a way to ignore those packages that collide in name but are not plugins. This proposal tries to be as agnostic as possible, not very Pythonic, but I am open to suggestions.
Checklist
./script/formatand./script/testlocally to ensure this change passes linter check and testExpected behavior
When the CLI searches for plugins, those packages that have a name
cz_but lack thediscover_thisattribute, will be ignored and the user will get a warningSteps to Test This Pull Request
cz_. It can be located under asrcfoldercommitizencommitizen changelog --help. Without this patch, an exception will be risen.Additional context