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

Add a default Event to dispatch events with just a name #40592

Merged
merged 1 commit into from
Jan 23, 2023

Conversation

phil-davis
Copy link
Contributor

@phil-davis phil-davis commented Jan 23, 2023

Description

In Symfony 5, eventDispatcher->dispatch must be passed an Event as the first parameter, followed by the event name.
In Symfony 4, an event can be dispatched with just a name (as the first parameter) or can have an Event and name (the Symfony 5 call interface)

This PR adjusts the remaining eventDispatcher->dispatch calls so that thet all pass an event as the first parameter.

Related Issue

#39630

How Has This Been Tested?

CI

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Database schema changes (next release will require increase of minor version instead of patch)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests only (no source changes)

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation ticket raised:
  • Changelog item, see TEMPLATE

@owncloud owncloud deleted a comment from update-docs bot Jan 23, 2023
@phil-davis phil-davis force-pushed the default-event-for-symfony-dispatch branch from 3f1a132 to 3c1ffc6 Compare January 23, 2023 03:58
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

66.7% 66.7% Coverage
0.0% 0.0% Duplication

@phil-davis phil-davis marked this pull request as ready for review January 23, 2023 04:41
@DeepDiver1975
Copy link
Member

So updating the dispatch to Symfony 5 will break all apps which dispatch events ..... well so be it ....

@DeepDiver1975 DeepDiver1975 merged commit cad960c into master Jan 23, 2023
@delete-merged-branch delete-merged-branch bot deleted the default-event-for-symfony-dispatch branch January 23, 2023 08:14
@phil-davis
Copy link
Contributor Author

So updating the dispatch to Symfony 5 will break all apps which dispatch events ..... well so be it ....

Yes, the order of parameters to dispatch changes in Symfony5. In Symfony4 they made the code accept the parameters in both the new and the old orders.

If/when we actually go to Symfony5, we first should get the dispatch calls adjusted in all apps, and get those apps released. They will then work with Symfony4 (and 5).

"Some time later" we can bump core to Symfony5.

I am trying to find the obvious things that need adjusting in core now, and they can be in 10.12. That is a small step forward.

There is security support for Symfony4 to Nov 2023, so we have time to gradually sort this out.

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.

2 participants