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 support for Symfony 6; drop < 5.1 #1239

Merged
merged 6 commits into from
Feb 17, 2022
Merged

Conversation

andrewmy
Copy link
Contributor

@andrewmy andrewmy commented Feb 10, 2022

  • Allow Symfony 6 in composer deps
  • Add Symfony 6 to the build matrix only for PHP 8
  • Bump min php-cs-fixer to ^3.4 to allow symfony/console 6, update its config
  • Bump min psr/log to 1.1
  • Add TestLogger from psr/log:1 because it's removed from 2 and 3
  • Fix Event Dispatcher compatibility for async events which happened to use a deprecated dispatcher configuration method
  • Drop Symfony older than 5.1

@andrewmy andrewmy marked this pull request as ready for review February 10, 2022 18:54
@andrewmy andrewmy marked this pull request as draft February 10, 2022 20:08
@Steveb-p
Copy link
Contributor

Steveb-p commented Feb 11, 2022

❤️ the commit history 😅

@andrewmy
Copy link
Contributor Author

Currently investigating the issue with async event listeners being registered twice — either an issue with the fixture app, or Sf 6 🤔

@andrewmy andrewmy marked this pull request as ready for review February 15, 2022 19:58
@andrewmy
Copy link
Contributor Author

andrewmy commented Feb 15, 2022

@makasim take a look pls.

Squashed everything to avoid letting all this commit message nonsense into master ;)

@andrewmy
Copy link
Contributor Author

Screenshot 2022-02-15 at 22 02 27

Screenshot 2022-02-15 at 22 02 37

Posting the commit message history for posterity :D

@makasim
Copy link
Member

makasim commented Feb 16, 2022

I see some juggling with event dispatcher.
Could dropping Symfony 4 support simplify things ?
If so, let's do it.

@makasim
Copy link
Member

makasim commented Feb 16, 2022

@andrewmy thank you for the great work!

@andrewmy
Copy link
Contributor Author

The new dispatcher thing was introduced in 5.1, so dropping means a bit wider scope. I'm not against it, your call :)

@makasim
Copy link
Member

makasim commented Feb 16, 2022

Let's keep it simple. Could you set "symfony/xxx": "^5.1|^6"

@Steveb-p
Copy link
Contributor

If it means a new major release, then we could include #1230 in it.

@andrewmy
Copy link
Contributor Author

@Steveb-p bumping minimum deps can happen in a patch version as long as the external API is unchanged.
In this case there is a slight difference in configuration — tricky case.

@andrewmy andrewmy changed the title Add support for Symfony 6 Add support for Symfony 6; drop < 5.1 Feb 16, 2022
@andrewmy
Copy link
Contributor Author

Used this opportunity to burn down whatever I found regarding the < 5 BC layer.

There's still the thing about one different line in fixture apps — session. Looks ugly but not warranting instant drop of anything < 5.4 I guess.

@makasim
Copy link
Member

makasim commented Feb 16, 2022

If it means a new major release,

Nah, it's a hell of a lot of work. just an ordinary release.

@makasim
Copy link
Member

makasim commented Feb 16, 2022

LGTM, could be merged?

@andrewmy
Copy link
Contributor Author

Yes please, with a pretty tag on top 🙏

@makasim makasim merged commit f366c60 into php-enqueue:master Feb 17, 2022
@andrewmy andrewmy deleted the add-sf-6 branch February 17, 2022 08:04
@andrewmy
Copy link
Contributor Author

Could we have a release? :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants