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
Updates Symfony components to 5.4.x #18328
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm surprised you have no BC from the Symfony Event Dispatcher as the main function (dispatch()) got a BC break. Lucky you 😇
Thanks for the comments @mickaelandrieu. This PR and the symfony updates are now scheduled for the 5.0.0 milestone as our initial look suggests it will require BC breaks. |
If you don't want this PR to be closed automatically in 28 days then you need to assign the label 'Do not close'. |
d4c7bd6
to
f91efbd
Compare
Updated the PR so it will require Symfony 5.4 instead. Once I've fixed all tests, this one should be ready to review. |
This issue is in "needs review" but there has been no activity for 7 days. ping @matomo-org/core-reviewers |
This issue is in "needs review" but there has been no activity for 7 days. ping @matomo-org/core-reviewers |
This issue is in "needs review" but there has been no activity for 7 days. ping @matomo-org/core-reviewers |
d68dbbd
to
a6546ca
Compare
a6546ca
to
40f875a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to to me. Automated tests are 100%, can't see any issues with the code and was able to run a selection of console commands with and without input with no issues, return self::SUCCESS
👍
- Functional review done
- Potential edge cases thought about (behavior of the code with strange input, with strange internal state or possible interactions with other Matomo subsystems)
- Usability review done (is anything maybe unclear or think about anything that would cause people to reach out to support)
- Security review done
- Wording review done
- Code review done
- Test updated Tests were added if useful/possible
- Has breaking changes, change log updated Reviewed for breaking changes
- Developer changelog updated if needed
- n/a Documentation added if needed
- n/a Existing documentation updated if needed
Description:
We are currently still using Symfony Console (and other components) in version 2.6.
This one is quite outdated and has some issues with PHP 8.1 (deprecation warnings).
This PR will update all components to the last available version 5.4.x.
So far there were two issues I have identified after the update:
The monolog class
ConsoleFormatter
does not longer have multiple constructor parameters, but takes an option array instead. This was adjusted inplugins/Monolog/config/cli.php
The
execute
method of a console command is now kind of forced to return an integer. Before that was kind of expected, but all other values were silently treated as0
. I have adjusted our command class, so it will catch and log that error and return0
in that case. That way commands that may not yet be adjusted won't fail. I have nevertheless changed all commands in core here and already created PRs for the submodule plugins.The DialogHelper has been deprecated and removed meanwhile. I've added it back for BC, but we need to migrate our usage to the QuestionsHelper instead.
Review