Allow to provide a default value for plugin options #3808
Conversation
rpunkfu
force-pushed
the
specify-default-option-values
branch
from
4769401
to
da105b3
Compare
|
@rpunkfu |
rpunkfu
force-pushed
the
specify-default-option-values
branch
from
da105b3
to
da9d7c2
Compare
|
I fixed that linting error @horike37, thanks for pointing it out :) |
rpunkfu
force-pushed
the
specify-default-option-values
branch
from
da9d7c2
to
4d994b8
Compare
rpunkfu
force-pushed
the
specify-default-option-values
branch
from
4d994b8
to
d9dcd73
Compare
| @@ -352,6 +353,13 @@ class PluginManager { | |||
| }); | |||
| } | |||
|
|
|||
| assignDefaultOptions(command) { | |||
| _.forEach(command.options, (value, key) => { | |||
| if (value.default && (!this.cliOptions[key] || this.cliOptions[key] === true)) { | |||
There was a problem hiding this comment.
I might be boolean-lost here, but why do we need this.cliOptions[key] === true condition?
There was a problem hiding this comment.
It was in options validation and I wanted them to be compatible :p
https://github.com/rpunkfu/serverless/blob/d9dcd73e5daf61302610d739e89ecece1a382552/lib/classes/PluginManager.js#L287
There was a problem hiding this comment.
lol #guilty 😂 ... @pmuens would be great if you could shed some light here
There was a problem hiding this comment.
😂 passing the ball to @dougmoscrop 😬 since he implemented it (according to the blame) --> https://github.com/rpunkfu/serverless/blame/d9dcd73e5daf61302610d739e89ecece1a382552/lib/classes/PluginManager.js#L287
There was a problem hiding this comment.
these_accusations === false && these_accusations !== true
I have no idea how it got there :D I did a lot of cut pasting when I did that refactor.....
There was a problem hiding this comment.
😂 thanks for the response @dougmoscrop.
So I would say that we can get rid of the double check since they really both check the same.
There was a problem hiding this comment.
There was a problem hiding this comment.
😬 Thanks @dougmoscrop --> ⛹️
So as the author of this weird piece of code I'd say that we can drop 😂
There was a problem hiding this comment.
looks like this is going further than I thought 😄 ... it needs further investigation and testing 😊
What did you implement:
Functionality that allows user to specify default values for plugin options.
Closes #2242
How did you implement it:
Options without a value will be assigned a default value, if it's specified.
How can we verify it:
Specifying default in any of the options, ensures that it will be that option's value, unless we set it manually via cli.
Todos:
Is this ready for review?: YES
Is it a breaking change?: NO