-
-
Notifications
You must be signed in to change notification settings - Fork 249
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
Respect variables with no value provided #1195
Conversation
When resolving a value for a variable, if a value has not been provided and it has no default then do not include it instead of setting it to `null`.
case value: Value => value | ||
for { | ||
definition <- variableDefinitions.find(_.name == name) | ||
value <- variableValues.get(name).orElse(definition.defaultValue) |
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.
nit 💅 : getOrElse
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.
definition.defaultValue
is an Option
so getOrElse
is not applicable?
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.
Yeah I think you're right
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.
But so is variableValues.get(name)
since you're getting it from a map, right?
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.
@frekw getOrElse
expects an A
, but defaultValue
is an Option[A]
. It's orElse
, not getOrElse
.
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.
I meant getOrElse
from Map :) https://scastie.scala-lang.org/jywLcoZARNG8KOrI7LDHqw
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.
But in either case, doesn't matter much.
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.
No, b
is of type Any
if you do that :P
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.
😬
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.
And also 🤦 I guess
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.
LGTM! ⭐
Thanks! |
When resolving a value for a variable, if a value has not been provided and it has no default then do not include it instead of setting it to `null`.
When resolving a value for a variable, if a value has not been provided and it has no default then do not include it instead of setting it to `null`.
When resolving a value for a variable, if a value has not been provided
and it has no default then do not include it instead of setting it to
null
.