Skip to content

Allow Symfony EventDispatcher::dispatch hooks to recurse#1678

Merged
morrisonlevi merged 4 commits into
masterfrom
levi/symfony-event-recurse
Aug 1, 2022
Merged

Allow Symfony EventDispatcher::dispatch hooks to recurse#1678
morrisonlevi merged 4 commits into
masterfrom
levi/symfony-event-recurse

Conversation

@morrisonlevi

@morrisonlevi morrisonlevi commented Jul 29, 2022 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Description

Related to #1105 and #1677, this allows the EventDispatcher::dispatch
to recurse. Historically this was not possible, and spans for various
events weren't captured because of this. Now that it is possible, we
should allow it. Tests will need to be fixed up.

Readiness checklist

  • Changelog has been added to the release document.
  • Tests added for this feature/bug.

Reviewer checklist

  • Appropriate labels assigned.
  • Milestone is set.

@morrisonlevi morrisonlevi added 🏆 enhancement A new feature or improvement cat:integration labels Jul 29, 2022
@morrisonlevi morrisonlevi added this to the future milestone Jul 29, 2022
@morrisonlevi

morrisonlevi commented Aug 1, 2022 •
edited
Loading

Copy link
Copy Markdown
Collaborator Author

I investigated the symfony.route.action changing in some of the error test. It's because this method is called more than once since the 'controller' event callback triggers more than once now. I was hoping to really quickly submit this in my free time this weekend, but this means I won't be able to.

@bwoebi
bwoebi force-pushed the levi/symfony-event-recurse branch from 4b72b17 to 94e4ea4 Compare August 1, 2022 10:28
@morrisonlevi
morrisonlevi marked this pull request as ready for review August 1, 2022 23:36
@morrisonlevi
morrisonlevi requested a review from a team as a code owner August 1, 2022 23:36
@morrisonlevi

morrisonlevi commented Aug 1, 2022

Copy link
Copy Markdown
Collaborator Author

Thanks, Bob. Updating the action for the first controller that runs seems like the best thing to do for now. Thanks for doing all the busy work on the rest of the spans!

@morrisonlevi morrisonlevi modified the milestones: future, 0.77.0 Aug 1, 2022
bwoebi
bwoebi approved these changes Aug 1, 2022
View reviewed changes

@bwoebi bwoebi left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Sounds like it looks good to you, so let's merge it?

@morrisonlevi morrisonlevi changed the title Allow Symfony EventDispatcher::dispatch to recurse Allow Symfony EventDispatcher::dispatch hooks to recurse Aug 1, 2022
@morrisonlevi
morrisonlevi merged commit 60afe63 into master Aug 1, 2022
@morrisonlevi
morrisonlevi deleted the levi/symfony-event-recurse branch August 1, 2022 23:44
@morrisonlevi morrisonlevi mentioned this pull request Aug 2, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bwoebi bwoebi bwoebi approved these changes

Assignees

No one assigned

Labels

cat:integration 🏆 enhancement A new feature or improvement

Projects

None yet

Milestone

0.77.0

Development

Successfully merging this pull request may close these issues.

2 participants

@morrisonlevi @bwoebi