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

Remove the listener priorities configuration options #407

Merged

Conversation

ste93cry
Copy link
Collaborator

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 added this to the 4.0 milestone Jan 10, 2021
@ste93cry ste93cry requested a review from Jean85 January 10, 2021 21:27
@ste93cry ste93cry force-pushed the remove-event-listener-priority-config-params branch 2 times, most recently from 78f01ec to 017a700 Compare January 10, 2021 21:40
@ste93cry ste93cry force-pushed the remove-event-listener-priority-config-params branch from 017a700 to 180cf64 Compare January 10, 2021 21:41
CHANGELOG.md Outdated Show resolved Hide resolved
UPGRADE-4.0.md Show resolved Hide resolved
composer.json Outdated Show resolved Hide resolved
@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.

None yet

3 participants