X Tutup
Skip to content

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

Merged
DeepDiver1975 merged 1 commit intomasterfrom
default-event-for-symfony-dispatch
Jan 23, 2023
Merged

Add a default Event to dispatch events with just a name#40592
DeepDiver1975 merged 1 commit intomasterfrom
default-event-for-symfony-dispatch

Conversation

@phil-davis
Copy link
Contributor

@phil-davis phil-davis commented Jan 23, 2023 •
edited
Loading

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

sonarqubecloud bot commented Jan 23, 2023

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

DeepDiver1975 commented Jan 23, 2023

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

phil-davis commented Jan 23, 2023

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

Reviewers

@DeepDiver1975 DeepDiver1975 DeepDiver1975 approved these changes

@jvillafanez jvillafanez Awaiting requested review from jvillafanez

Assignees

No one assigned

Labels

3 - To Review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@phil-davis @DeepDiver1975
X Tutup