-
Notifications
You must be signed in to change notification settings - Fork 9
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
add semistrictbool for fixing booleans #91
Conversation
pycfmodel/resolver.py
Outdated
@@ -19,7 +20,7 @@ def _extended_bool(value) -> bool: | |||
|
|||
|
|||
class _BooleanModel(BaseModel): | |||
bool_value: bool | |||
bool_value: SemiStrictBool |
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.
this is not strictly necessary, it can reveted
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.
Check 15863aa since got it reverted and test pass.
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.
Talked with @oscarbc96 , approving on his behalf
pycfmodel/model/types.py
Outdated
return value | ||
|
||
if isinstance(value, str) and value.lower() in ("true", "false"): | ||
return value == "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.
What about if value is "True"
or "TRUE"
? Do we need to have value.lower() == "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.
Made a proposal on how to solve this at df58525
@oscarbc96 can you check that possible solution as well?
Thank you both!
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.
Talked with @oscarbc96 , approving on his behalf
0.16.3 - [2022-02-24]
Fixes
resolve
forbool
s that can bestr
such as"true"
or"false"
or similar, by makingResolvableBool
to be resolvable toSemiStrictBool
.Updates
CLOUDFORMATION_ACTIONS
.