Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork5.3k
Use the short Yaml syntax for service definition#7860
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
| app.twig_extension: | ||
| class:AppBundle\Twig\AppExtension | ||
| AppBundle\Twig\AppExtension: | ||
| public: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.
I hesitated to use_defaults here to be able to remove this line and use the short syntax, wdyt?
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'd say, let's not add the "defaults" until we decide what to do with that new option.
service_container/tags.rst Outdated
| AppBundle\Twig\AppExtension: | ||
| tags:[twig.extension] | ||
| versionadded:: 3.3 |
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.
To makeversionadded a RST directive, you must include the two.. at the beginning:.. versionadded::
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.
Indeed, thanks. I also fixed it a few lines above.
service_container/tags.rst Outdated
| ..versionadded::3.3 | ||
| Support for the short syntax for service definition in the YAML format | ||
| was introduced in Symfony 3.3. |
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 think this should be moved toservice_container, after#7807 is merged. I did not have that note there.
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 agree I didn't realize I added it intags.rst... I remove it and paste it here to not lose it.
..tip:: In YAML format, you may define a service with a simple array of tags as long as you don't need additional attributes. The following definitions are equivalent. ..code-block::yamlservices:# Compact syntaxAppBundle\Twig\AppExtension:[twig.extension]# Verbose syntaxAppBundle\Twig\AppExtension:tags:[twig.extension] ..versionadded::3.3 Support for the short syntax for service definition in the YAML format was introduced in Symfony 3.3.
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.
Actually rethinking about it, does it make sense? We don't talk about tags inservice_container.
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.
Don't we already have a place where we explain the difference?
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 don't think so, it was requested by#7441.
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.
Hm, not sure if we really need 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.
Won't this be confusing for people who try to use it in lower versions because of our exemples using this syntax ?
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 we talk about different things? Do you mean theversionadded directive? Otherwise I don't see the problem as the changes were only merged into themaster branch.
xabbuh commentedMay 4, 2017
Thank you@GuilhemN. |
Uh oh!
There was an error while loading.Please reload this page.
Fix#7441