Skip to content

[Notifier] Add symfony/event-dispatcher to bridges' require-dev section #48548

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

Closed
wants to merge 1 commit into from

Conversation

OskarStark
Copy link
Contributor

@OskarStark OskarStark commented Dec 8, 2022

Q A
Branch? 6.3 (not sure)
Bug fix? no
New feature? no
Deprecations? no
Tickets n/a
License MIT
Doc PR n/a

symfony/event-dispatcher is used in all tests.

Also increase some php requirements to >=8.1

Not sure if this is the right branch

@carsonbot
Copy link

Hey!

Thanks for your PR. You are targeting branch "6.3" but it seems your PR description refers to branch "6.3 (not sure)".
Could you update the PR description or change target branch? This helps core maintainers a lot.

Cheers!

Carsonbot

@carsonbot carsonbot added this to the 6.3 milestone Dec 8, 2022
@OskarStark OskarStark force-pushed the move-event-dispatcher branch from 51243ff to 2d42a02 Compare December 9, 2022 09:08
…ection

Also increase some php requirements to `>=8.1` and remove `ext-json` from `require` section as its always available since PHP 8

Not sure if this is the right branch
@OskarStark OskarStark force-pushed the move-event-dispatcher branch from 2d42a02 to 0424c41 Compare December 9, 2022 09:11
@OskarStark OskarStark requested a review from fabpot December 9, 2022 09:31
@fabpot
Copy link
Member

fabpot commented Dec 9, 2022

Why? Tests pass without this change, right?

@OskarStark
Copy link
Contributor Author

Maybe because symfony/notifier, which is required in every bridge, requires symfony/event-dispatcher-contracts in require section? 🤔

@OskarStark
Copy link
Contributor Author

So how do we proceed here?

@nicolas-grekas

@nicolas-grekas
Copy link
Member

Adding event-dispatcher is not needed since it's not used in tests.

I've applied the other changes you spotted from 5.4 to 6.3, see ad3424c for the last merge up.

@OskarStark
Copy link
Contributor Author

Thank you

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.

4 participants