Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
Kernel::$projectDir is inconsistent#22727
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
xabbuh commentedMay 17, 2017
I am 👎 on this change. Maintaining BC for protected properties is a nightmare. And you can already achieve the same by overriding the |
javiereguiluz commentedMay 17, 2017
@leofeyer allowing to override these variables is very important ... but we don't allow to do that changing the value of the properties directly, but by overriding the So I'm going to close this issue as "won't fix" because this can already be solved in other way. Thanks! |
stof commentedMay 17, 2017
Note that writing directly to |
stof commentedMay 17, 2017
thus, you still have not provided a use case requiring the visibility change (i.e. something whichcannot be achieved by overwriting the method) |
leofeyer commentedMay 17, 2017
Ok, thanks for the clarification. We should merge#22728 then. |
This PR was merged into the 3.3 branch.Discussion----------[HttpKernel] Fix kernel.project_dir extensibility| Q | A| ------------- | ---| Branch? | 3.3| Bug fix? | yes| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets |#22727| License | MIT| Doc PR | n/aAlternative to#22727 that makes use of the existing public api.Commits-------3230fc7 Fix kernel.project_dir extensibility
This PR was merged into the 3.3 branch.Discussion----------[HttpKernel] Fix kernel.project_dir extensibility| Q | A| ------------- | ---| Branch? | 3.3| Bug fix? | yes| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets |symfony/symfony#22727| License | MIT| Doc PR | n/aAlternative to #22727 that makes use of the existing public api.Commits-------3230fc7 Fix kernel.project_dir extensibility
The new
Kernel::$projectDirproperty isprivateinstead ofprotected, which is inconsistent if you want to set the property in a child class.Since the Kernel class is almost always extended and since the project root dir is very likely to be overwritten (e.g. in the unit tests), the property should be
protectedIMHO.