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.

Sign up for GitHub

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

Jump to bottom

Spring gardening: Symfony 7 support #762

Merged
merged 17 commits into from
Apr 11, 2024
Merged

Spring gardening: Symfony 7 support #762

merged 17 commits into from
Apr 11, 2024

Conversation

michael-rubel
Copy link
Contributor

@michael-rubel michael-rubel commented Mar 12, 2024 •
edited
Loading

About

  • Updates all the dependencies alongside Symfony Console v7.
  • Updates tests, configurations, and fixes CI issues.

Symfony 7 is required for seamless usage with Laravel 11.

@Ocramius Ocramius added enhancement WIP dependencies Pull requests that update a dependency file labels Mar 12, 2024
@michael-rubel
Copy link
Contributor Author

Hey @Ocramius

Can we do anything to allow symfony/console ^7.0 without breaking the CI, or would you rather address the newly reported issues?

@Ocramius
Copy link
Member

Ocramius commented Mar 12, 2024 •
edited
Loading

@michael-rubel I suggest trying a very minimal composer update symfony/console to reduce composer.lock changes.

Also, let's please bump the range: don't widen it.

Ocramius
Ocramius reviewed Mar 12, 2024
View reviewed changes
composer.lock Outdated Show resolved Hide resolved
@michael-rubel
Copy link
Contributor Author

Yeah, I tried that minimal update at first but psalm and some other packages want to upgrade before it installs v7.

@Ocramius
Copy link
Member

Would suggest helping with #732 first then :)

@michael-rubel
Copy link
Contributor Author

michael-rubel commented Mar 12, 2024 •
edited
Loading

It looks like the bot already tried the update in #753

The #732 is a bit stale. It probably makes sense to bump the lock to the latest versions and then address the issues, because for example psalm is required at least from 5.16 as it's the version when they added support for Symfony 7 (ref: https://github.com/vimeo/psalm/releases/tag/5.16.0).

@Ocramius
Copy link
Member

That was already attempted in #730: needs work :)

@michael-rubel
Copy link
Contributor Author

Hell, I'll see if I have spare time for this.

@Ocramius
Copy link
Member

The project needs some spring gardening work, basically :D

Ocramius
Ocramius reviewed Mar 22, 2024
View reviewed changes
.gitignore Outdated Show resolved Hide resolved
@michael-rubel
Copy link
Contributor Author

@Ocramius It seems that everything passes on PHP 8.2+

@michael-rubel michael-rubel marked this pull request as ready for review March 22, 2024 23:12
@michael-rubel michael-rubel changed the title Allow symfony/console ^7.0 and update dependencies Spring gardening: Symfony 7 support Mar 22, 2024
Ocramius
Ocramius reviewed Mar 22, 2024
View reviewed changes
Copy link
Member

@Ocramius Ocramius left a comment

Choose a reason for hiding this comment

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

Looking good! I haven't dug in depth, since I'm multitasking like crazy, but I think we should hunt down the last few suppressions first.

I know it's annoying, but it takes so many days to recover from going below threshold @_@

src/Change.php Outdated Show resolved Hide resolved
src/DetectChanges/BCBreak/FunctionBased/ParameterDefaultValueChanged.php Outdated Show resolved Hide resolved
src/SourceLocator/LocatedSourceWithStrippedSourcesDirectory.php Show resolved Hide resolved
@michael-rubel
Copy link
Contributor Author

@Ocramius Ping 👋

@Ocramius
Copy link
Member

@michael-rubel I need to let this sit for a bit: currently stuck with pre-easter-vacation-deadlines-everywhere. Do ping again if stuck after Easter!

@michael-rubel
Copy link
Contributor Author

@Ocramius how's it going? 😁

@Ocramius
Copy link
Member

Only one failure, and it's not really part of the public API of this package anyway: nice!

Error: The number of required arguments for Symfony\Component\Console\Command\Command#setApplication() increased from 0 to 1

Let's 🚢 this

@Ocramius Ocramius added this to the 8.7.0 milestone Apr 11, 2024
@Ocramius Ocramius removed the WIP label Apr 11, 2024
@Ocramius Ocramius merged commit 92d33e4 into Roave:8.7.x Apr 11, 2024
10 of 12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Ocramius Ocramius Ocramius left review comments

Assignees

@michael-rubel michael-rubel

Labels
dependencies Pull requests that update a dependency file enhancement
Projects
None yet
Milestone
8.7.0
Development

Successfully merging this pull request may close these issues.
2 participants
@michael-rubel @Ocramius