Skip to content

[DependencyInjection][Improve] Constant YAML with expression language #7395

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

Closed
wants to merge 2 commits into from

Conversation

20uf
Copy link
Contributor

@20uf 20uf commented Jan 21, 2017

Hi here,

Ï notice two things about this documentation.
http://symfony.com/doc/current/service_container/parameters.html#constants-as-parameters

@javiereguiluz Is it an oversight or is there a reason that is not specified ?
Can I add this documentation that you have indicated in your news ?

Regards,

Add example constant YAML with expression language
@20uf 20uf changed the title [dependency_injection][Improve] Constant YAML with expression language [DependencyInjection][Improve] Constant YAML with expression language Jan 21, 2017
@20uf
Copy link
Contributor Author

20uf commented Feb 13, 2017

Hello @javiereguiluz @xabbuh ,

Concerning this PR, do we agree on the substance?

Regards,

@GuilhemN
Copy link
Contributor

As there is already !php/const I don't think we should document this, it would only be misleading imo.

@xabbuh
Copy link
Member

xabbuh commented Feb 17, 2017

We could add a YAML code block to the configuration block, make use of the expression there, but add a caution below that you must have the ExpressionLanguage component installed. We should then also add a tip that this is supported natively by the YAML parser starting with 3.2 and then update the example after merging up to not make use of the expression anymore.

@javiereguiluz
Copy link
Member

@xabbuh but we already document "PHP constants in YAML" in 9635f0f#diff-122786a4391045b1aa3edea4ab2e676f ... maybe we shouldn't care about that in this PR ... which by the way, should be merged in 2.7 instead of 3.2, right?

@xabbuh
Copy link
Member

xabbuh commented Feb 19, 2017

@javiereguiluz Yeah, I think this should be done for 2.7 and 2.8 where you couldn't use the native parser feature.

@20uf 20uf changed the base branch from 3.2 to 2.7 February 22, 2017 12:13
@20uf 20uf changed the base branch from 2.7 to 3.2 February 22, 2017 12:19
@20uf
Copy link
Contributor Author

20uf commented Feb 22, 2017

Hi,

Does NOT merge that into 3.x.
New PR is open for 2.7 & 2.8. => Move on #7520

Regards,

@20uf 20uf closed this Feb 22, 2017
@20uf 20uf deleted the patch-3 branch February 22, 2017 14:50
xabbuh added a commit that referenced this pull request Feb 28, 2017
…ion language] (20uf, javiereguiluz)

This PR was merged into the 2.7 branch.

Discussion
----------

[DependencyInjection][Improve] Constant YAML with expression language]

Hi,

Continuation of the PR #7395 for the 2.7 branch.

Regards,

Commits
-------

1dc3913 Fixed the RST syntax
6d0de9d Minor rewords
79191bf [DependencyInjection][Improve] Constant YAML with expression language]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants