-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Fixed zero exit code when plugin not found #1122
Fixed zero exit code when plugin not found #1122
Conversation
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.
From the tests, I see that missing backends were purposefully allowed. Before merging this, it would be great to check the reason and if someone relies on this behavior.
Otherwise ✔️
I noticed that too, but I'm honestly not sure who I should ask. |
I am not sure either. @Michael-F-Bryan do you have an idea? |
I guess an alternative could be to hide it behind a flag, like |
I'm surprised that missing backends are allowed. I guess one reason you may want that is if the Instead of a command line flag, you could add an extra item to the |
It surprised me as well. It seems like a design decision that's definitely going to bite someone at some point (and it seems like it did). I like the idea with the Maybe @ehuss wants to decide? :) |
Or maybe @Dylan-DPC ? |
looks good to me. .will wait for a review from @ehuss |
What about adding an
Or maybe
|
We'd need to update the docs so 3rd party backends (and preprocessors?) know about the convention and don't try to use the key as part of their config, although I doubt that happens in practice at the moment anyway. |
Thank you for your input. How does this look? The default behavior is to require the backend and to stop if it's missing. Adding |
Seems ok. Can you add documentation? And rebase on master. |
I wrote a little blurb in the docs, but I'm not sure how to go about rebasing to master. |
363cdca
to
2732c5e
Compare
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.
Thanks! I made a few wording tweaks, and squashed the changes.
…e-when-plugin-not-found-893 Fixed zero exit code when plugin not found
In response to issue #893.