Conversation
…ing values. The following values are interpreted as if a positive answer was meant and will return a boolean True: True, 1 and these strings in any casing: "y", "yes", "t", "true", "on". Everything else is considered False.
|
@thet thanks for creating this Pull Request and helping to improve Plone! TL;DR: Finish pushing changes, pass all other checks, then paste a comment: To ensure that these changes do not break other parts of Plone, the Plone test suite matrix needs to pass, but it takes 30-60 min. Other CI checks are usually much faster and the Plone Jenkins resources are limited, so when done pushing changes and all other checks pass either start all Jenkins PR jobs yourself, or simply add the comment above in this PR to start all the jobs automatically. Happy hacking! |
|
@jenkins-plone-org please run jobs |
ale-rt
left a comment
ale-rt
left a comment
There was a problem hiding this comment.
@thet did you see plone/plone.app.theming#256?
plone/plone.app.theming#245 (comment) was merged without an approval.
For me, the name of the function you are proposing does not match what the function is doing.
For what is worth, please check: https://lmarena.ai/c/b230e92d-a9ab-4610-831a-06453243cde7 or https://lmarena.ai/c/3220aec1-6470-40b2-8547-b2b3a2e2d942
IMO is_affirmative or is_truthy are better names.
In addition, there are some minor changes that should be applied:
- The docstring is mentioning lower case
trueandfalse - The function is not always returning a
boolean - Looking up in sets is much more efficient
All of that is solved in plone/plone.app.theming#256
|
I was also thinking of asking for a better function name. Starting with |
|
@ale-rt I haven't seen it yet - I will move the code over here as plone.base is a better place for generally useful utilities. |
|
@jenkins-plone-org please run jobs |
4 participants
Related:
plone/Products.CMFPlone#4188
plone/Products.CMFPlone#4169
#85
plone/plonetheme.barceloneta#404
plone/plone.app.portlets#203
The following values are interpreted as if a positive answer was meant and will return a boolean True:
True, 1 and these strings in any casing: "y", "yes", "t", "true", "on".
Everything else is considered False.
...
I need something like this now for the second time (first was in: plone/plone.app.theming#245 ) and it has the potential to unify the usage of yes/no values in generic setup importexport handlers (which is IIRC not unified).
Btw. this is inspired by but simplified: https://github.com/mxstack/mxdev/blob/main/src/mxdev/vcs/common.py#L109