Skip to content
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

[RFR] Added "How to Organize Configuration Files" cookbook #3885

Merged
merged 9 commits into from
Aug 16, 2014
Merged

[RFR] Added "How to Organize Configuration Files" cookbook #3885

merged 9 commits into from
Aug 16, 2014

Conversation

javiereguiluz
Copy link
Member

Q A
Doc fix? no
New docs? yes
Applies to 2.3+
Fixed tickets #3680

This is just the first draft of a new cookbook. Please, tell me anything that I should add, change or remove. Thanks!

@javiereguiluz javiereguiluz changed the title Firest draft of the "How to Organize Configuration Files" cookbook First draft of the "How to Organize Configuration Files" cookbook May 28, 2014

# app/config/config.yml
imports:
- { resource: 'bundles/' }
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@wouterj
Copy link
Member

wouterj commented May 28, 2014

Great job! I'm thinking about writing some articles about how to migrate from the standard SE to a fully custom SE. I think this is a great start of that idea! I'll review it later this week.

@@ -0,0 +1,258 @@
.. index::
single: Configuration, Environments
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we want to refer people to this article in the "Environments" index, do we?

@wouterj
Copy link
Member

wouterj commented May 29, 2014

This article should also be added to cookbook/map.rst.inc

├─ vendor/
└─ web/

This default structure was choosen for its simplicity — one file per environment.
Copy link
Member

Choose a reason for hiding this comment

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

chosen

@javiereguiluz
Copy link
Member Author

@cordoval @wouterj @stof thanks for your reviews! I've added all your suggestions.

@javiereguiluz javiereguiluz changed the title First draft of the "How to Organize Configuration Files" cookbook [RFR] New "How to Organize Configuration Files" cookbook Jun 2, 2014
@javiereguiluz javiereguiluz changed the title [RFR] New "How to Organize Configuration Files" cookbook [RFR] Added "How to Organize Configuration Files" cookbook Jun 2, 2014
}

Following the same technique explained in the previous section, make sure to
load the appropriate configuration files from each main file (``common.yml``,
Copy link
Member

Choose a reason for hiding this comment

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

load -> import to be consistent with the terms used in the code?

.. code-block:: text

<your-project>/
├─ app/
Copy link
Contributor

Choose a reason for hiding this comment

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

we could also remove the /'s since anything that lacks extension is a folder? although some executables lack extension

Copy link
Member

Choose a reason for hiding this comment

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

I think including the slash makes things easier to understand

Copy link
Member

Choose a reason for hiding this comment

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

👍 For keeping the slash for readability reasons.

Copy link
Member

Choose a reason for hiding this comment

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

👍 as well. Not all files have an extension (app/console for instance, even if it does not appear in this tree)

@cordoval
Copy link
Contributor

cordoval commented Jun 3, 2014

good job 👍

@javiereguiluz
Copy link
Member Author

@cordoval, thanks for your review. I have included all your suggestions except these ones because I don't know what does the style guide say about them: /instead of your-project/, using trailing / for dir names and adding the . in file extensions.

Regarding the ClosureLoader, don't you think it would be too complex to add it to this article?

file to execute your own logic. In addition, you can define your own services
to load configuration from databases and web services.

Directory Loading
Copy link
Member

Choose a reason for hiding this comment

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

Does directory loading really works out of the box ? I don't see anything about it in the codebase. Are you using a thrid-party bundle for that ?

Copy link
Member Author

Choose a reason for hiding this comment

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

@stof you are right. Thanks for noticing this. I was confused by the resource loading of the Config component and the fact that this correctly works for loading routes defined as annotations in one or more controllers inside a bundle.

I've just removed this section.

Copy link
Member

Choose a reason for hiding this comment

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

I suggest you open a feature request on Symfony for this. It looks useful (in case you don't need to strictly control the order of loading)

Copy link
Contributor

Choose a reason for hiding this comment

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

@javiereguiluz, I did a PR about Directory Loading, if you want to take a look: symfony/symfony#11059

{
// ...

public function registerContainerConfiguration(LoaderInterface $loader)
Copy link
Member

Choose a reason for hiding this comment

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

Either a use statement for the LoaderInterface or // ... should be added to the top of the file.

Copy link
Member

Choose a reason for hiding this comment

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

The same applies to the Kernel class.

@weaverryan
Copy link
Member

This reads great! It fell of my radar, but we've added some new labels to track finished PR's (Wouter did all the work), which makes finished PR's easier to focus on. Progress! Thanks!

@weaverryan weaverryan merged commit 4241ee5 into symfony:2.3 Aug 16, 2014
weaverryan added a commit that referenced this pull request Aug 16, 2014
…ook (javiereguiluz)

This PR was merged into the 2.3 branch.

Discussion
----------

[RFR] Added "How to Organize Configuration Files" cookbook

| Q             | A
| ------------- | ---
| Doc fix?      | no
| New docs?     | yes
| Applies to    | 2.3+
| Fixed tickets | #3680

This is just the first draft of a new cookbook. Please, tell me anything that I should add, change or remove. Thanks!

Commits
-------

4241ee5 Removed redundant configuration blocks
68a7d2a Added more suggestions made by @xabbuh
be414e0 Removed the "Directory Loading" section because it doesn't work out of the box
da3e1d0 Fixed the target of one cookbook article link
af5d478 Added suggestions made by @cordoval
4a87cfb Fixed the link to another cookbook article
2a32b23 Added the latest suggestions made by reviewers
d861be3 Added all the suggestions made by reviewers
0520bf1 Firest draft of the "How to Organize Configuration Files" cookbook
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