Skip to content

fix(deps)!: Upgrade symfony/event-dispatcher to supported version 5.4.26 - #38546

Merged
nickvergessen merged 1 commit into
masterfrom
techdebt/noid/upgrade-symfony/event-dispatcher
Aug 10, 2023
Merged

fix(deps)!: Upgrade symfony/event-dispatcher to supported version 5.4.26#38546
nickvergessen merged 1 commit into
masterfrom
techdebt/noid/upgrade-symfony/event-dispatcher

Conversation

@nickvergessen

@nickvergessennickvergessen commented May 31, 2023•
edited
Loading

Copy link
Copy Markdown
Member

Summary

TODO

Checklist

@nickvergessennickvergessen added 2. developing Work in progress technical debt 🧱 🤔🚀 labels May 31, 2023
@nickvergessennickvergessen added this to the Nextcloud 28 milestone May 31, 2023
@nickvergessennickvergessen self-assigned this May 31, 2023
@nickvergessen
nickvergessen marked this pull request as draft May 31, 2023 12:28
@nickvergessennickvergessen mentioned this pull request May 31, 2023
Merged
@nickvergessennickvergessen changed the title Techdebt/noid/upgrade symfony/event dispatcherfix(deps): Upgrade symfony/event-dispatcher to supported version 5.4.22May 31, 2023
@nickvergessennickvergessen changed the title fix(deps): Upgrade symfony/event-dispatcher to supported version 5.4.22fix(deps)!: Upgrade symfony/event-dispatcher to supported version 5.4.22Jun 2, 2023
@nickvergessen
nickvergessenforce-pushed the techdebt/noid/upgrade-symfony/event-dispatcher branch from a408dcf to 01f4507CompareJune 2, 2023 07:58
@nickvergessennickvergessen added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Jun 2, 2023
github-advanced-security[bot]
github-advanced-securityAI found potential problems Jun 2, 2023
View reviewed changes
Comment threadapps/workflowengine/lib/Manager.php Fixed
Comment threadapps/workflowengine/lib/Manager.php Fixed
Comment threadapps/workflowengine/lib/Manager.php Fixed
@nickvergessen
nickvergessenforce-pushed the techdebt/noid/upgrade-symfony/event-dispatcher branch from 5e598e1 to d4f4997CompareJune 2, 2023 10:51
@nickvergessen
nickvergessenforce-pushed the techdebt/noid/upgrade-symfony/event-dispatcher branch from d4f4997 to 5befe72CompareJune 23, 2023 07:13
@nickvergessen

nickvergessen commented Jun 23, 2023

Copy link
Copy Markdown
MemberAuthor

There are hundreds of psalm errors now because our legacy SymfonyAdapter claims to be a Symfony\Component\EventDispatcher\EventDispatcherInterface and it's typed in all kind of places like that, but at the same time the order change Symfony announced.

I see 2 ways forward at the moment:

  1. Swap the order of the event and eventName everywhere and continue
    • breaks only emitting code, so apps are only affected when they create events, not when they listen to events
  2. move to typed events
    • breaks listening apps too

@nickvergessen
nickvergessenforce-pushed the techdebt/noid/upgrade-symfony/event-dispatcher branch from 5befe72 to 10fdcfaCompareJune 23, 2023 08:40
@ChristophWurst

ChristophWurst commented Jun 27, 2023

Copy link
Copy Markdown
Member

here are hundreds of psalm errors now because our legacy SymfonyAdapter claims to be a Symfony\Component\EventDispatcher\EventDispatcherInterface and it's typed in all kind of places like that, but at the same time the order change Symfony announced.

Deprecated in 20 and was never a public API. I'm fine dropping it and only maintaining the OCP dispatcher with 28+

Ref #21848

This was referenced Jun 30, 2023
Merged
Merged
Merged
This was referenced Jul 11, 2023
Merged
Merged
This was referenced Jul 19, 2023
Merged
Merged
Merged
This was referenced Jul 27, 2023
Merged
Merged
@nickvergessen
nickvergessenforce-pushed the techdebt/noid/upgrade-symfony/event-dispatcher branch from 10fdcfa to d35fe04CompareJuly 28, 2023 08:54
@nickvergessennickvergessen mentioned this pull request Jul 28, 2023
Merged
@nickvergessen
nickvergessenforce-pushed the techdebt/noid/upgrade-symfony/event-dispatcher branch from d35fe04 to 9511c81CompareJuly 28, 2023 15:11
github-advanced-security[bot]
github-advanced-securityAI found potential problems Jul 28, 2023
View reviewed changes
Comment threadapps/files_external/lib/Service/BackendService.php
static $eventSent = false;
if (!$eventSent) {
\OC::$server->getEventDispatcher()->dispatch(
\OC::$server->get(IEventDispatcher::class)->dispatch(

Check notice

Code scanning / Psalm

DeprecatedMethod

The method OCP\EventDispatcher\IEventDispatcher::dispatch has been marked as deprecated
@nickvergessennickvergessen mentioned this pull request Jul 28, 2023
Merged
4 tasks
@nickvergessennickvergessen changed the title fix(deps)!: Upgrade symfony/event-dispatcher to supported version 5.4.22fix(deps)!: Upgrade symfony/event-dispatcher to supported version 5.4.26Aug 4, 2023
@nickvergessen
nickvergessenforce-pushed the techdebt/noid/upgrade-symfony/event-dispatcher branch from 9511c81 to 327e37aCompareAugust 4, 2023 09:49
@nickvergessen
nickvergessen marked this pull request as ready for review August 4, 2023 09:50
ChristophWurst
ChristophWurst approved these changes Aug 4, 2023
View reviewed changes

@ChristophWurstChristophWurst left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

🐩

kesselb
kesselb approved these changes Aug 4, 2023
View reviewed changes

@kesselbkesselb left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice 👏

@juliusknorrjuliusknorr mentioned this pull request Aug 7, 2023
Merged
4 tasks
@nickvergessen
fix(deps)!: Upgrade symfony/event-dispatcher to supported version 5.4.26
63b6fb7
Signed-off-by: Joas Schilling <coding@schilljs.com>
@nickvergessen

nickvergessen commented Aug 10, 2023

Copy link
Copy Markdown
MemberAuthor

Rebasing to see if the failing acceptance tests are linked or temporary

@nickvergessen
nickvergessenforce-pushed the techdebt/noid/upgrade-symfony/event-dispatcher branch from 327e37a to 63b6fb7CompareAugust 10, 2023 07:36
@nickvergessen
nickvergessen merged commit 9533dcf into masterAug 10, 2023
@nickvergessen
nickvergessen deleted the techdebt/noid/upgrade-symfony/event-dispatcher branch August 10, 2023 09:48
@nickvergessennickvergessen mentioned this pull request Oct 26, 2023
Merged
@OrchalOrchal mentioned this pull request Jan 12, 2024
Open
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@AndyScherzingerAndyScherzingerAndyScherzinger approved these changes
@ChristophWurstChristophWurstChristophWurst approved these changes
@AltahrimAltahrimAltahrim approved these changes
@kesselbkesselbkesselb approved these changes
@come-nccome-ncAwaiting requested review from come-nc

Assignees

@nickvergessennickvergessen

Labels

3. to reviewWaiting for reviewstechnical debt🧱 🤔🚀

Projects

None yet

Milestone

Nextcloud 28

Development

Successfully merging this pull request may close these issues.

6 participants

@nickvergessen@ChristophWurst@AndyScherzinger@Altahrim@kesselb@github-advanced-security