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

Updated the value of path option to match the official recipe #912

Merged
merged 1 commit into from Feb 3, 2019

Conversation

javiereguiluz
Copy link
Contributor

This problem was spotted by @nicolas-grekas. He suggested to use instead the same value as in the official recipe (https://github.com/symfony/recipes/blob/master/doctrine/doctrine-bundle/1.6/manifest.json). I fully agree with that and I hope you do too. Thanks.

Copy link
Member

@greg0ire greg0ire left a comment

Choose a reason for hiding this comment

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

Besides consistency, here is a good reason to make this change:

bin/console debug:container --parameter kernel.data_dir
2019-01-30T23:45:09+01:00 [error] Error thrown while running command "debug:container --parameter 'kernel.data_dir'". Message: "You have requested a non-existent parameter "kernel.data_dir". Did you mean one of these: "kernel.root_dir", "kernel.cache_dir", "kernel.logs_dir"?"

In ParameterBag.php line 100:
                                                                                                                                                       
  You have requested a non-existent parameter "kernel.data_dir". Did you mean one of these: "kernel.root_dir", "kernel.cache_dir", "kernel.logs_dir"? 

@greg0ire
Copy link
Member

Also git log -S data_dir in symfony/symfony only yields symfony/symfony@9e4aebf and symfony/symfony@1a45bb6 , as if that directory never was a thing… weird

Copy link
Member

@alcaeus alcaeus left a comment

Choose a reason for hiding this comment

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

Thanks for improving @javiereguiluz! Could I ask you to rebase the PR against the 1.10 branch so we can fix the docs for the currently stable version? Thanks!

@javiereguiluz javiereguiluz changed the base branch from master to 1.10.x February 1, 2019 16:55
@javiereguiluz javiereguiluz changed the base branch from 1.10.x to master February 1, 2019 16:56
@javiereguiluz javiereguiluz changed the base branch from master to 1.10.x February 1, 2019 16:58
@javiereguiluz
Copy link
Contributor Author

@alcaeus I may have rebased this successfully ... but please double check it. Thanks.

@alcaeus alcaeus closed this Feb 3, 2019
@alcaeus alcaeus reopened this Feb 3, 2019
@alcaeus alcaeus self-assigned this Feb 3, 2019
@alcaeus alcaeus added this to the 1.10.2 milestone Feb 3, 2019
@alcaeus
Copy link
Member

alcaeus commented Feb 3, 2019

Thanks @javiereguiluz!

@alcaeus alcaeus merged commit a7f09da into 1.10.x Feb 3, 2019
@alcaeus alcaeus deleted the path-option branch February 3, 2019 10:06
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.

None yet

4 participants