-
Notifications
You must be signed in to change notification settings - Fork 75
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
#190-FetchInteger: Fix #492
base: main
Are you sure you want to change the base?
Conversation
807e6f5
to
4589846
Compare
@artiodev thanks so much. could you add some tests? |
Signed-off-by: Gianluca Artioli <[email protected]>
Signed-off-by: Gianluca Artioli <[email protected]>
7846b4f
to
f22b744
Compare
@yxxhero Done |
@mumoshu WDYT? |
ResultBool: "true" | ||
ResultInteger: "1" |
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.
@artiodev Thanks a bunch for the pull request!
This looks well aligned with vals' design, where it wants any fetched values to be rendered as strings, so that the values before(ref+...
) and after the vals rendering are both strings. It's great in that regard.
But, is this what you originally wanted in #190?
Here, I was wondering if you wanted:
ResultBool: "true" | |
ResultInteger: "1" | |
ResultBool: true | |
ResultInteger: 1 |
rather than the string representations of the original values.
If necessary, we could support both options, perhaps via feature toggling envvars and or ref URL params. Just wanted to confirm your goal and intention to start!
Thanks in advance for your cooperation 🙏
Hello @mumoshu, However at the moment this solution could be enough for me. Thanks! |
Hey @artiodev! Thanks for confirming! From vals' design perspective, I believe that both work if each is enabled only in an opt-in manner. Would you like it, if it looked like this:
We initially implement Does that make sense to you? cc/ @yxxhero |
yeah. it's better. |
yeah, This solution is even better |
#190
Handles int and bool types when returned as string map value.