Skip to content

Remove the listener priorities configuration options #407

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

Merged
merged 3 commits into from
Jan 11, 2021
Merged

Remove the listener priorities configuration options #407

merged 3 commits into from
Jan 11, 2021

Conversation

ste93cry
Copy link
Contributor

I know we discussed about removing these configuration options in the past, but I strongly believe that this kind of things should not be part of the bundle configuration mainly for these reasons:

  • There is already a way to change this kind of things, which is writing a compiler pass or overriding the service
  • I think that it's not a common thing to change this, in a lot of years of work with Symfony I rarely if never had to change the priority of a third-party listener
  • Exposing this configuration to users puts it on an equal footing with changing any bundle or SDK option, which is false because it affects the inner workings of the bundle itself
  • The listeners (for example the tracing listener I would like to implement in the near future) must run in a specific priority which correlates them. Letting users change such priorities will likely break things because they must know that they must also update all related priorities: at that point, you're falling into an advanced use-case which to me means that you also have the knowledge to implement the proper solution

@ste93cry ste93cry requested a review from Jean85 January 11, 2021 12:42
@Jean85 Jean85 merged commit 213d839 into getsentry:master Jan 11, 2021
@ste93cry ste93cry deleted the remove-event-listener-priority-config-params branch January 11, 2021 17:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants