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

Updates Symfony components to 5.4.x #18328

Merged
merged 19 commits into from Feb 14, 2023
Merged

Updates Symfony components to 5.4.x #18328

merged 19 commits into from Feb 14, 2023

Conversation

sgiehl
Copy link
Member

@sgiehl sgiehl commented Nov 16, 2021

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 in plugins/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 as 0. I have adjusted our command class, so it will catch and log that error and return 0 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

@sgiehl sgiehl mentioned this pull request Nov 16, 2021
38 tasks
@sgiehl sgiehl added this to the 5.0.0 milestone Nov 18, 2021
Copy link

@mickaelandrieu mickaelandrieu left a 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 😇

composer.json Outdated Show resolved Hide resolved
core/Console/DialogHelper.php Show resolved Hide resolved
core/Console/DialogHelper.php Show resolved Hide resolved
core/Plugin/ConsoleCommand.php Outdated Show resolved Hide resolved
plugins/CoreAdminHome/Commands/DeleteLogsData.php Outdated Show resolved Hide resolved
@justinvelluppillai
Copy link
Contributor

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.

@github-actions
Copy link
Contributor

If you don't want this PR to be closed automatically in 28 days then you need to assign the label 'Do not close'.

@github-actions github-actions bot added the Stale The label used by the Close Stale Issues action label Dec 19, 2021
@sgiehl sgiehl added Do not close PRs with this label won't be marked as stale by the Close Stale Issues action and removed Stale The label used by the Close Stale Issues action labels Dec 19, 2021
@sgiehl sgiehl changed the base branch from 4.x-dev to 5.x-dev June 20, 2022 14:33
@sgiehl sgiehl force-pushed the updatesymfony branch 2 times, most recently from d4c7bd6 to f91efbd Compare August 23, 2022 15:36
@sgiehl sgiehl changed the title Updates Symfony components to 5.3.x Updates Symfony components to 5.4.x Aug 23, 2022
@sgiehl
Copy link
Member Author

sgiehl commented Aug 23, 2022

Updated the PR so it will require Symfony 5.4 instead. Once I've fixed all tests, this one should be ready to review.

@sgiehl sgiehl added Needs Review PRs that need a code review and removed Do not close PRs with this label won't be marked as stale by the Close Stale Issues action labels Aug 25, 2022
@sgiehl sgiehl marked this pull request as ready for review August 25, 2022 10:27
@github-actions
Copy link
Contributor

github-actions bot commented Sep 2, 2022

This issue is in "needs review" but there has been no activity for 7 days. ping @matomo-org/core-reviewers

@github-actions github-actions bot added the Stale The label used by the Close Stale Issues action label Sep 2, 2022
@sgiehl sgiehl added Do not close PRs with this label won't be marked as stale by the Close Stale Issues action and removed Stale The label used by the Close Stale Issues action labels Sep 2, 2022
@github-actions
Copy link
Contributor

This issue is in "needs review" but there has been no activity for 7 days. ping @matomo-org/core-reviewers

@github-actions github-actions bot added Stale The label used by the Close Stale Issues action and removed Stale The label used by the Close Stale Issues action labels Sep 10, 2022
@github-actions
Copy link
Contributor

This issue is in "needs review" but there has been no activity for 7 days. ping @matomo-org/core-reviewers

@sgiehl sgiehl removed the Stale The label used by the Close Stale Issues action label Jan 30, 2023
@sgiehl sgiehl mentioned this pull request Jan 30, 2023
6 tasks
@sgiehl sgiehl force-pushed the updatesymfony branch 5 times, most recently from d68dbbd to a6546ca Compare February 7, 2023 09:40
Copy link
Contributor

@bx80 bx80 left a 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 👍

@sgiehl sgiehl merged commit c00da19 into 5.x-dev Feb 14, 2023
@sgiehl sgiehl deleted the updatesymfony branch February 14, 2023 08:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Review PRs that need a code review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants