-
Notifications
You must be signed in to change notification settings - Fork 1
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
fix: FIR-33174: parse boolean in old (0/1) and new (false/true) formats #86
Conversation
Minimum allowed coverage is Generated by 🐒 cobertura-action against 0b02579 |
64a0bde
to
400593d
Compare
public static bool ParseBoolean(string str) | ||
{ | ||
bool b; | ||
return booleanValues.TryGetValue(str, out b) ? b : throw new FormatException($"String '{str}' was not recognized as a valid Boolean."); |
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.
Should this be a case-insensitive comparison? For cases like True
and TRUE
?
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.
In fact it should support both true
and True
because string value arrives here after calling ToString()
that turns true
into string True
and false
into string False
.
I agree that this is not obvious, so I can either add this text as a comment or add entries for True
and False
to the dictionary. What do you prefer?
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 see. Eh, just a comment here is fine so we don't forget.
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
public static bool ParseBoolean(string str) | ||
{ | ||
bool b; | ||
return booleanValues.TryGetValue(str, out b) ? b : throw new FormatException($"String '{str}' was not recognized as a valid Boolean."); |
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 see. Eh, just a comment here is fine so we don't forget.
524d008
to
e2efab2
Compare
e2efab2
to
0b02579
Compare
Quality Gate passedIssues Measures |
No description provided.