Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Support typed parameter values #1376
Support typed parameter values #1376
Changes from 8 commits
749ae25
effeed1
61c93dd
3e7da70
bb35f03
13999e7
ac5703c
b51fb1a
46072b8
5d00d42
77a6eaa
bd94729
73f222a
48ba154
dbe000f
2b67286
3505640
9b0bbbf
ea8b0c7
a10760a
f7c7cbf
ee4a316
327e06a
82a47c2
cb920eb
91e69dc
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
shouldn't there be an error on the use of a toggle param that is NOT a string... which is technically a bool ?
It seems like we are missing that..
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.
Isn't this what this check does? We try to cast to a string, if it's
!ok
we error 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.
Isn't that done by the line below? If
stringVal
is neither "true" nor "false",strconv.ParseBool
will return an error.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.
The error message is somewhat confusing, we expect a string/boolean there. Maybe smth. like:
wdyt?
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 disagree, because this is checked in the line below when parsing the string to bool. This line is checking that
typeof(interface) == string
, not the content of the string.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.
If it's not a string it can't be
true
orfalse
which is what we expect.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.
Booleans can also be represented by a boolean or integer type. Also
strconv.ParseBool
accepts 1, t, T, TRUE, true, True, 0, f, F, FALSE, false, False.At this point it's only clear whether the input is a string or not, the content of the input isn't known yet.