-
-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Another collection of fixes: Typos, Formatting, Logic, Links #3174
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
Another collection of fixes: Typos, Formatting, Logic, Links #3174
Conversation
.. caution:: | ||
|
||
When using YAML, be sure to surround ``Null`` with quotes (``"Null"``) | ||
or else YAML will convert this into a Null value. |
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.
null
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.
Do you mean like ...or else YAML will convert this into a null
value?
@@ -45,7 +45,7 @@ method returns **false**: | |||
Acme\BlogBundle\Entity\Author | |||
getters: | |||
stateInvalid: | |||
- "False": | |||
- 'False': |
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.
Is this a standard? I'm more accustomed to seeing double-quotes in YAML. Or is it that double-quotes in YAML makes this still equate to a boolean (instead 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.
Me too, I'm more used to double quotes in this context. It is just to make it consistent between the True, False and Null validator descriptions. And no, a double quoted string does not evaluate to the Boolean value.
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.
👍 for using double quoted YAML strings here. The True and Null validators can be changed alike if this was necessary.
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 Yaml component uses single quotes to escape the reserved names in Yaml. I think we should use 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.
@wouterj Just wanted to check what the Yaml component does for reference ;-)
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.
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.
Weird, what's the reason for using single quotes?
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.
And you can see both in core :)
The same is true of config.yml
in the standard edition. So, since it doesn't matter, let's just be as consistent as we can. Since True and Null are using single quotes, let's just keep this.
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.
Currently, True and False are using double quotes - Null was not. So in the first version I changed Null to use double quotes. Now, in this PR, everything is updated to single quote. Since it doesn't matter, we can go ahead with single quotes.
…ogic_collection Another collection of fixes: Typos, Formatting, Logic, Links
Another summary of some minor issues in this PR.
Please comment if something is wrong - I'll do an update then.