Skip to content

Simplified the framework configuration reference #7201

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 5 commits into from
Nov 2, 2017

Conversation

javiereguiluz
Copy link
Member

I propose two main changes:

  • Show how can users display the default config and their own config.
  • Remove the dump of the default config values.

I never liked the config dump because in my opinion:

  • It simply adds "noise" and doesn't explain anything
  • It's hard to maintain it updated
  • The same can easily be achieved with a dedicated console command
  • Removing it will simplify the work of doc maintainers and it'll reduce the merge conflicts
  • These references are very useful for the config option descriptions, not for just listing those options

Thoughts? Thanks!

PS: if you agree, I'll update the rest of the references.

@ogizanagi
Copy link
Contributor

I actually like having the full configuration reference. It gives a big picture of the available features, its options and defaults, without having to boot a symfony app and using the config:dump-reference command (which sometimes may not work because you made a mistake in this very same configuration).

But that's only my opinion. I kind of understand the "It's hard to maintain it updated" argument, because we don't think to update it often enough and it shouldn't be updated manually, but it actually only requires pasting the result of the config:dump-reference command.

@javiereguiluz
Copy link
Member Author

I've reverted the most controversial changes in this PR, so now it just proposes to reword the intro and mention the debug:config and the config:dump-reference commands.

Copy link
Contributor

@HeahDude HeahDude left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great addition! 👍


.. tip::
# displays the default config values defined by Symfony
$ ./bin/console config:dump-reference framework
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be php bin/console ... (same below) to be Windows compatible

Copy link
Member

@weaverryan weaverryan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do like listing the config:dump command (btw, I usually shorten it - config:dump is so much nicer) on each reference page.

FrameworkBundle Configuration ("framework")
===========================================
FrameworkBundle Configuration
=============================
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you not like the ("framework") part? I always like it... because what the user really thinks is "what things can I put under this framework key?)

@javiereguiluz
Copy link
Member Author

I made all the asked changes. Ready to merge it. Thanks.

@weaverryan
Copy link
Member

Thanks Javier!

@weaverryan weaverryan merged commit a71ef9d into symfony:2.7 Nov 2, 2017
weaverryan added a commit that referenced this pull request Nov 2, 2017
…guiluz, weaverryan)

This PR was merged into the 2.7 branch.

Discussion
----------

Simplified the framework configuration reference

I propose two main changes:

* Show how can users display the default config and their own config.
* Remove the dump of the default config values.

I never liked the config dump because in my opinion:

* It simply adds "noise" and doesn't explain anything
* It's hard to maintain it updated
* The same can easily be achieved with a dedicated console command
* Removing it will simplify the work of doc maintainers and it'll reduce the merge conflicts
* These references are very useful for the config option descriptions, not for just listing those options

Thoughts? Thanks!

PS: if you agree, I'll update the rest of the references.

Commits
-------

a71ef9d Shortened command... easier to remember ;)
2d68dc3 Last fixes
b91b196 Restored the "Full Default Configuration" dump
396548c Fixed an indentation issue
8076c40 Simplified the framework configuration reference
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.

7 participants