Drop deprecated code from Bundles & Bridges #14127
Conversation
stloyd
commented
Mar 31, 2015
| Q | A |
|---|---|
| Bug fix? | no |
| New feature? | no |
| BC breaks? | yes |
| Deprecations? | no |
| Tests pass? | yes |
| License | MIT |
| @@ -16,6 +16,9 @@ | ||
|
|
||
| class ContainerAwareEventManagerTest extends \PHPUnit_Framework_TestCase | ||
| { | ||
| protected $container; | ||
| protected $evm; |
There was a problem hiding this comment.
use private vibility please (and this should be submitted separately as a bugfix as it should not go only in master)
There was a problem hiding this comment.
It was already done #14128.
There was a problem hiding this comment.
then it should not appear in this diff
e17700b to
a0e3da6
Compare
|
|
||
| private function getRequest() | ||
| { | ||
| if ($this->requestStack && $currentRequest = $this->requestStack->getCurrentRequest()) { |
There was a problem hiding this comment.
The requestStack property can't be null so no need to check if it's set.
Same goes for SessionHelper
a0e3da6 to
67903e7
Compare
|
@fabpot @nicolas-grekas how are we dealing with these removal PRs which are starting to come ? Are we letting @stloyd make a bug PR here, or are we dealing with many smaller PRs ? |
67903e7 to
f99d0f6
Compare
|
Less PRs about this topic, the better. If we can centralize everything in one PR, that would be even better. |
|
The hardest part will be keeping tests green on both 3.0 and 2.7 by not forgetting to update composer dependencies. See an example of this in #14216 |
|
Can you rebase? |
| @@ -72,26 +61,20 @@ public function hasFlash($name) | ||
| return $this->getSession()->getFlashBag()->has($name); |
There was a problem hiding this comment.
Request::getSession says @return SessionInterface|null so this is not save to be called.
|
Closing this old PR which is now obsolete and does too many things. |
|
@fabpot What is the way to go with the removal of deprecated features? Small PRs for each bundle/component/bridge? I'm asking because this is the oposite of what did you say in a previous comment |
|
@dosten One PR would have been great but it must be ready to be merged. As this one is quite old and because we merged so many removal since then, it would make rebasing this PR difficult. The best now is indeed to do small PRs for one component or one bundle. |