[FrameworkBundle] HttpKernel properties, visibility from private to protected #3942
Conversation
|
when ever submitting a PR to reduce visibility restrictions its important to mention a use case. these restrictions are put in place intentionally to ensure that we steer API users in the right direction for extension points. |
|
actually wondering if we should have a dedicated section in the contribution docs for these kinds of PR's |
|
They're useful if one want to add support for additional "standalone: type". |
|
@lsmith77 - anything that helps repeating the same thing over and over. To be honest, there needs to be some guidelines about how to word PR's and commit messages - that's lacking. Here is something we did for Zikula: https://sites.google.com/a/zikula.org/development/Guideines/commit-guidelines |
|
I ran into the same problem with the base HttpKernel in Drupal. I'd suggest going even further down the stack, frankly. Use case: In Drupal right now, our kernel implementation subclasses from HttpKernel and overrides only the constructor, at fabpot's recommendation. We call the parent constructor, but then want to make use of $this->dispatcher and $this->resolver. Those are private, so we cannot. Instead, we have to duplicate the properties, call the parent constructor, and then reset the properties in the subclass. That's just silly: dispatcher = $dispatcher; $this->resolver = $resolver; // ... ?>If those properties were protected, that wouldn't be necessary. In fact I don't think I've ever run into a case where I felt a private member variable was appropriate in favor of protected. (OK, not the exact code in question in this PR, but a related case.) |
|
@Crell the issue when using protected properties everywhere is that we must maintain BC for all of them as we don't know if someone is using it in an extended class. This would make it impossible to refactor any internal code |
Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
Related to: #3904
Please allow usage of these properties in child classes.