-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[EventDispatcher] remove deprecated features #22796
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
[EventDispatcher] remove deprecated features #22796
Conversation
xabbuh
commented
May 20, 2017
Q | A |
---|---|
Branch? | master |
Bug fix? | no |
New feature? | no |
BC breaks? | yes |
Deprecations? | no |
Tests pass? | yes |
Fixed tickets | |
License | MIT |
Doc PR |
0e66009
to
dbd7e50
Compare
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.
The following assignment
$deprecationWhitelist = array('event_dispatcher' => ContainerAwareEventDispatcher::class); |
@@ -7,7 +7,7 @@ | |||
<services> | |||
<defaults public="false" /> | |||
|
|||
<service id="event_dispatcher" class="Symfony\Component\EventDispatcher\ContainerAwareEventDispatcher" public="true"> | |||
<service id="event_dispatcher" class="Symfony\Component\EventDispatcher\EventDispatcher" public="true"> | |||
<argument type="service" id="service_container" /> |
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.
does this argument not require changes?
0eb15ca
to
a7fae50
Compare
Looks like 3.4 doesn't like this change - forward layer missing? |
see #22814 |
This PR was merged into the 4.0-dev branch. Discussion ---------- [FrameworkBundle] FC with EventDispatcher 4.0 | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #22796 (comment) | License | MIT | Doc PR | Commits ------- b7c76f7 [FrameworkBundle] FC with EventDispatcher 4.0
This PR was merged into the 4.0-dev branch. Discussion ---------- [FrameworkBundle] FC with EventDispatcher 4.0 | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | symfony/symfony#22796 (comment) | License | MIT | Doc PR | Commits ------- b7c76f7 [FrameworkBundle] FC with EventDispatcher 4.0
This PR was merged into the 4.0-dev branch. Discussion ---------- [FrameworkBundle] FC with EventDispatcher 4.0 | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #22796 (comment) | License | MIT | Doc PR | Commits ------- b7c76f7 [FrameworkBundle] FC with EventDispatcher 4.0
This PR was merged into the 4.0-dev branch. Discussion ---------- [FrameworkBundle] FC with EventDispatcher 4.0 | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | symfony/symfony#22796 (comment) | License | MIT | Doc PR | Commits ------- b7c76f7 [FrameworkBundle] FC with EventDispatcher 4.0
a7fae50
to
598bec4
Compare
builds are green, ready to be reviewed |
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.
👍
@@ -309,7 +300,6 @@ public function load(array $configs, ContainerBuilder $container) | |||
'Symfony\\Component\\DependencyInjection\\Container', | |||
|
|||
'Symfony\\Component\\EventDispatcher\\Event', | |||
'Symfony\\Component\\EventDispatcher\\ContainerAwareEventDispatcher', |
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 think you should revert this line: it will only create a conflict with that other PR removing the block
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.
without this change it would add to class cache non-existent class. Maybe better merge #22786 first
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.
ok merged, asis, #22786 already has conflicts to fix
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.
@Koc Actually, that would not have been an issue. This part of the code is never executed on PHP 7 (which is a requirement for Symfony 4).
Thank you @xabbuh. |
This PR was merged into the 4.0-dev branch. Discussion ---------- [EventDispatcher] remove deprecated features | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | no | BC breaks? | yes | Deprecations? | no | Tests pass? | yes | Fixed tickets | | License | MIT | Doc PR | Commits ------- 598bec4 [EventDispatcher] remove deprecated features