Move %locale% to config.yml #776
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,7 @@ | ||
| # This file is a "template" of what your parameters.yml file should look like | ||
| # Set parameters here that may be different on each deployment target of the app, e.g. development, staging, production. | ||
| # http://symfony.com/doc/current/best_practices/configuration.html#infrastructure-related-configuration | ||
| parameters: | ||
| database_host: 127.0.0.1 | ||
| database_port: ~ | ||
| database_name: symfony | ||
|
|
@@ -14,7 +15,5 @@ parameters: | |
| mailer_user: ~ | ||
| mailer_password: ~ | ||
|
|
||
| # A secret key that's used to generate certain security-related tokens | ||
| secret: ThisTokenIsNotSoSecretChangeIt | ||
|
There was a problem hiding this comment. As we discussed in symfony/symfony#14026 the secret MUST NOT be different for each machine. So this stays in contrast to the added comment at the top of the file. There was a problem hiding this comment. Thanks Tobion - I see the comment here: symfony/symfony#14026 (comment) I don't think this should hold up this PR, though - I think moving the secret somewhere else should be a different issue. There was a problem hiding this comment. @Tobion the secret must be the same for all server of a given deployment setup (i.e. all your prod servers). they need to share the secret. but it is the same for all other settings (if each prod server use a different DB, it could create weird stuff). There was a problem hiding this comment. correct, then I would suggest to rephrase the comment to "Set parameters here that may be different on each deployment target of the app, e.g. development, staging, production." There was a problem hiding this comment. 👍 for updating it to that exact wording |
||
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 would also move the
database_driverhere. You don't change the database driver on each machineThere 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.
That makes sense, I like the idea of moving that as well.
One minor concern to think about: Do you think that harms the getting-started experience for new users with a different environment set-up?
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.
@MaxvanderVelde good point, I think we have to make the distinction between 2 kinds of install:
The first one is what the symfony installer should take care of, and it includes this kind of parameters.
The second one is what the Incenteev ParameterHandler takes care of, and it includes the parameters like database username and password.
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 database_driver should be moved as well to be correct.
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.
Not sure if database_driver really should be a parameter at all then.
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 generally configure it directly indeed
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.
Same, if it's considered as app-relative, having it directly in
parametersmight be useless.By the way, maybe it can break BC, depending whether it's merged in master or 2.6/7 ? I don't know if this parameter is used elsewhere by service or conf, etc.
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.
+1 for removing the
database_driverparameter altogether and configuring it directly. It should make usingsqlitea little bit easier I think anyways. I don't think this is a BC break, since the SE is just for new projects (and nothing internally currently uses thedatabase_driverparameter anyways).So, let's do it!
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.
+1 for moving the database_driver as well.