Skip to content
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

Split events v7 #287

Merged
merged 10 commits into from
Dec 31, 2019
Merged

Split events v7 #287

merged 10 commits into from
Dec 31, 2019

Conversation

rpkamp
Copy link
Contributor

@rpkamp rpkamp commented Dec 20, 2019

Q A
Bug fix no
New feature yes
BC breaks no
Deprecations yes
Tests pass yes
Fixed tickets N/A
License MIT

PR to add functionality introduced in #284, but based against version 7 of this bundle, including deprecation notices.

Symfony 4.4 doesn't ship with tests anymore as of version 4.4, so we're
not able to use their logger for testing this bundle against 4.4.

With our own Logger for the tests we can test against Symfony 4.4 again.
There will be a lot of tests added to this class over the following
commits, and it would benefit the readability of the class as a whole if
it is cleaned up a bit.
Instead of listening to all pre transaction events and
filtering for a desired service name, clients can not also listen for
events from specific clients.
Instead of listening to all post transaction events and
filtering for a desired service name, clients can not also listen for
events from specific clients.
@gregurco gregurco self-requested a review December 26, 2019 14:48
@rpkamp
Copy link
Contributor Author

rpkamp commented Dec 27, 2019

All feedback processed, @gregurco 🙂

Copy link
Member

@gregurco gregurco left a comment

Choose a reason for hiding this comment

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

Great 👍

@gregurco
Copy link
Member

gregurco commented Dec 29, 2019

@florianpreusner please check this one too 🙂

Copy link
Member

@florianpreusner florianpreusner left a comment

Choose a reason for hiding this comment

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

👍

@gregurco gregurco merged commit b61e820 into 8p:v7 Dec 31, 2019
@rpkamp rpkamp deleted the split-events-v7 branch December 31, 2019 16:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants