Skip to content

Explain sample configuration parameter usage #5621

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

Merged
merged 1 commit into from
Apr 15, 2017
Merged

Conversation

bennyt2
Copy link
Contributor

@bennyt2 bennyt2 commented Aug 12, 2015

The "Processing the $configs Array" section explains how to define parameters within the Configuration class and process them in the Extension. In my opinion, it doesn't take the example to a logical conclusion. People reading this file want to learn how to create configuration within a Bundle. It would be helpful to explain how the configuration can be used. Since the example uses parameters, I propose showing how the parameters can be loaded into the container. With these code changes, the twitter.client_id and twitter.client_secret are now available through use throughout the application.

The "Processing the ``$configs`` Array" section explains how to define parameters within the Configuration class and process them in the Extension. In my opinion, it doesn't take the example to a logical conclusion. People reading this file want to learn how to create configuration within a Bundle. It would be helpful to explain how the configuration can be used. Since the example uses parameters, I propose showing how the parameters can be loaded into the container. With these code changes, the `twitter.client_id` and `twitter.client_secret` are now available through use throughout the application.
@xabbuh
Copy link
Member

xabbuh commented Aug 17, 2015

Even though I think that you have a point in saying that we fail to explain how to work with a processed configuration, I don't agree with the proposed solution. The reason is that exposing parameters makes them globally available in the whole application. This makes it harder to make future changes as you cannot remove parameters to maintain backwards compatibility. Instead, I suggest to show how to conditionally load services based on actual configuration values or to dynamically inject constructor/method arguments into service definitions.

@weaverryan weaverryan merged commit ceacc7c into symfony:2.7 Apr 15, 2017
weaverryan added a commit that referenced this pull request Apr 15, 2017
This PR was merged into the 2.7 branch.

Discussion
----------

Explain sample configuration parameter usage

The "Processing the `$configs` Array" section explains how to define parameters within the Configuration class and process them in the Extension. In my opinion, it doesn't take the example to a logical conclusion. People reading this file want to learn how to create configuration within a Bundle. It would be helpful to explain how the configuration can be used. Since the example uses parameters, I propose showing how the parameters can be loaded into the container. With these code changes, the `twitter.client_id` and `twitter.client_secret` are now available through use throughout the application.

Commits
-------

ceacc7c Explain sample configuration parameter usage
@weaverryan
Copy link
Member

Merged! Thanks @bennyt2 - I think your point is totally valid :). I enhanced the example at sha: 8292f92 like @xabbuh suggested.

Cheers!

xabbuh added a commit that referenced this pull request Apr 15, 2017
* 2.7: (31 commits)
  Fixed the RST syntax
  Improve example context
  [#5621] Enhancing example of using bundle config
  [#7601] minor tweak
  Update expiration.rst
  Update expiration.rst
  Update expiration.rst
  Update expiration.rst
  Minor reword and fixed the line length
  Improve specification explanation
  [#7664] minor wording tweak
  Rewords and minor fixes
  Add an explanation about «constraints» validation
  [#7645] enumerate ordered list items implicitly
  Adding a new article about "Creating a Bug Reproducer"
  Fixed a syntax issue
  Use backticks
  #7311 choice_value callback argument can be null
  Fixed broken links for nginx & FastCgiExternalServer
  Update dialoghelper.rst
  ...
xabbuh added a commit that referenced this pull request Apr 15, 2017
* 2.8: (46 commits)
  [#7507] fix component name
  [#7490] minor typo fix
  Added a note about redirections to absolute URLs in tests
  Added the changes suggested by reviewers
  [#7620] use generate() in PHP templates before 2.8
  Fixed the RST syntax
  Improve example context
  [#5621] Enhancing example of using bundle config
  [#7601] minor tweak
  Update expiration.rst
  Update expiration.rst
  Update expiration.rst
  Update expiration.rst
  Minor reword and fixed the line length
  Improve specification explanation
  [#7664] minor wording tweak
  Rewords and minor fixes
  Add an explanation about «constraints» validation
  [#7645] enumerate ordered list items implicitly
  Adding a new article about "Creating a Bug Reproducer"
  ...
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.

3 participants