Symfony 6 support (without Flex dependency) #1402

loic425 · 2021-12-01T10:40:05Z

No description provided.

ciaranmcnulty · 2021-12-01T15:18:59Z

These Psalm issues may have been resolved in the #1397 branch, and that commit could be cherry-picked

loic425 · 2021-12-02T15:52:43Z

@ciaranmcnulty I was looking the errors on the CI and I was wondering if you want to keep the support of php < 8.0.
Cause Symfony 6 has dropped support for PHP 7.4 and you override some Symfony console methods.

webignition · 2021-12-07T10:32:25Z

Symfony 6 supports PHP >= 8.0.2, support for Symfony 6 requires dropping support for PHP < 8.0.2.

It's not really a question of whether you want to drop support for PHP < 8 but more of a realisation that this is, as far as I understand, dropping support for PHP < 8 is an unavoidable requirement.

In order to support Symfony 6, is there any other choice other than to drop support for PHP < 8?

loic425 · 2021-12-07T11:10:36Z

@webignition Yes I think this is ok with keeping PHP 7 support. I think I was wrong with the issue encountered.

loic425 · 2021-12-15T09:09:17Z

@ciaranmcnulty Could you approve running the CI build please?

webignition · 2021-12-17T10:42:08Z

src/PhpSpec/Console/Command/RunCommand.php

@@ -31,7 +32,7 @@
 */
 final class RunCommand extends Command
 {
-    public function getApplication() : Application
+    public function getApplication() : ?BaseApplication


This method needs to return an instance of PhpSpec\Console\Application instead of Symfony\Component\Console\Application due to needing to subsequently call getContainer() which is a method of the former but not a method of the latter.

Is this change necessary? I'm not saying it isn't, just curious given the impact this change makes.

I've tried this out in my own attempt to get Symfony 6.0 working and I can't see the need for this.

Maybe I have to try again without these change...
But this is a different return typehint than the Symfony one and I'm not sure it can work.
Maybe just an assertion can be enough on phpspec code.

Returning PhpSpec\Console\Application not only works but is necessary for the command to call PhpSpec\Console\Application::getContainer().

You are correct in saying that the return type is different. An instance of PhpSpec\Console\Application is acceptable to return here as it is a child type of (i.e. extends) Symfony\Component\Console\Application. Technically speaking, this is a covariant return type and support was introduced in PHP 7.4.

Yes you're right. But "we" have to drop support for PHP 7.3 then.

And I think we can drop support for Symfony 3 too... It is not maintained anymore.

@ciaranmcnulty Are you ok to drop both PHP 7.3 and Symfony 3 support?

The PHP 7.3 concern is a valid point that I completely failed to consider and that does complicate things somewhat. But with PHP 7.3 now being EOL, I don't think it's fully unwise to consider dropping support.

I think we should keep this returning Application, then do a runtime assertion in the code that it's a BaseApplication we got back

I think we can keep the existing code code, cause on PHP lower than 8.0 we have not symfony 6 downloaded so this still works with no return type on symfony lower than 6.

loic425 · 2021-12-22T09:05:46Z

CI should be fixed before with that PR and Psalm error too

shouze · 2022-01-30T15:36:46Z

@webignition @ciaranmcnulty do you think guys this PR can get merged soon, I can help if any other blocker 👼

webignition · 2022-01-31T15:10:02Z

@shouze There's nothing in particular that I can do to help speed things along other than suggest what might need to happen next.

Which is, in the following order:

merge Run psalm against PHP version that is being used by workflow #1408 to prevent psalm from trying to check PHP 8.x code against PHP 7.x (@ciaranmcnulty)
rebase this branch against master (@loic425)
kick off the build checks for this PR (@ciaranmcnulty)
address any issues if the checks for this PR don't pass

@ciaranmcnulty Does this seem like a good approach?

loic425 · 2022-01-31T15:47:24Z

@webignition I would add the PHP 8.1 support PR between first and second steps

loic425 · 2022-03-07T22:51:42Z

@ciaranmcnulty I will work on this PR this week now you merged the other needed PRs. Thx a lot

loic425 · 2022-03-08T07:46:50Z

@ciaranmcnulty Do you really want to maintain PHP 7.3 support?
Please look at this thread #1402 (comment).

But if we keep PHP 7.3 support, we have to change this typehint and adding an extra check on the method.

loic425 · 2022-03-09T16:30:08Z

@ciaranmcnulty Could you run the CI on this one please?
Let's move forward an bring that support to the whole Symfony community.

ciaranmcnulty · 2022-03-09T19:17:58Z

I don't mind dropping 7.3 support, it'll just mean a new major (only because that's the convention we established) - it should drop unmaintained Symfony too

loic425 · 2022-03-09T19:58:05Z

I don't mind dropping 7.3 support, it'll just mean a new major (only because that's the convention we established) - it should drop unmaintained Symfony too

Great, in this current PR? CI is green for other PHP versions.

ciaranmcnulty · 2022-03-11T21:24:26Z

I've updated the next branch to reflect the minimum PHP and Symfony versions we'll support in 8.0.0.

Can you target this PR against that branch please?

ciaranmcnulty · 2022-03-12T10:32:49Z

I got another chance to think about this; I think we should support PHP 7.3. The next major will be 8.0+ and that will leave Symfony users who want to use SF 6.0 but are stuck on PHP 7.4 without a version to use.

(commented inline in code how we can support 7.3)

loic425 · 2022-03-13T06:17:21Z

I got another chance to think about this; I think we should support PHP 7.3. The next major will be 8.0+ and that will leave Symfony users who want to use SF 6.0 but are stuck on PHP 7.4 without a version to use.

(commented inline in code how we can support 7.3)

Do you have an Idea to solve the last error. It seems to need some investigation but this is not related to this PR imho.

ciaranmcnulty · 2022-03-13T09:08:53Z

Do you have an Idea to solve the last error. It seems to need some investigation but this is not related to this PR imho.

It's fixed on main, please rebase

loic425 · 2022-03-13T17:19:44Z

It's fixed on main, please rebase

done

ciaranmcnulty · 2022-03-13T20:30:01Z

Thank you, I plan to do a release of PhpSpec 7 and then 8 (from next branch)

loic425 · 2022-03-14T07:41:59Z

Thank you too @ciaranmcnulty 🎉

loic425 mentioned this pull request Dec 1, 2021

Add support for Symfony 6 #1399

Closed

webignition mentioned this pull request Dec 1, 2021

Use Symfony 6.0 smartassert/users#221

Closed

GeoDpto mentioned this pull request Dec 6, 2021

Support for symfony 6 php-http/stopwatch-plugin#17

Closed

loic425 mentioned this pull request Dec 15, 2021

Add support for Symfony 5 and 6 FriendsOfBehat/ExcludeSpecificationsExtension#26

Merged

webignition reviewed Dec 17, 2021

View reviewed changes

loic425 force-pushed the symfony-6-support branch from 4b87b9e to 04628c7 Compare December 22, 2021 12:31

Zales0123 mentioned this pull request Jan 5, 2022

Add Symfony 6 support winzou/state-machine#49

Merged

loic425 mentioned this pull request Jan 12, 2022

Symfony 6 support Sylius/Sylius#13274

Closed

lchrusciel mentioned this pull request Jan 14, 2022

PHPSpec Support PHP 8.1 #1406

Closed

shouze mentioned this pull request Feb 9, 2022

Fix PHP 8.1 errors #1407

Merged

loic425 force-pushed the symfony-6-support branch from 45a4efa to 9f5e246 Compare March 8, 2022 07:43

ciaranmcnulty changed the base branch from main to next March 11, 2022 21:24

ciaranmcnulty changed the base branch from next to main March 12, 2022 10:31

loic425 and others added 4 commits March 13, 2022 18:19

Symfony 6 support (without Flex dependency)

57747e0

Fix typehint and drop support for PHP 7.3

0be38aa

Downgrade PHP to 7.3 on composer

5dd3088

[Console] Fix finder defintion in PHPSpec

47fe0d1

loic425 force-pushed the symfony-6-support branch from dcf9fa2 to 47fe0d1 Compare March 13, 2022 17:19

ciaranmcnulty merged commit 717b82d into phpspec:main Mar 13, 2022

loic425 deleted the symfony-6-support branch March 14, 2022 07:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Symfony 6 support (without Flex dependency) #1402

Symfony 6 support (without Flex dependency) #1402

loic425 commented Dec 1, 2021

ciaranmcnulty commented Dec 1, 2021

loic425 commented Dec 2, 2021

webignition commented Dec 7, 2021

loic425 commented Dec 7, 2021

loic425 commented Dec 15, 2021

webignition Dec 17, 2021

webignition Dec 17, 2021 •

edited

loic425 Dec 22, 2021

webignition Dec 22, 2021

loic425 Dec 22, 2021

loic425 Dec 22, 2021

loic425 Dec 22, 2021

webignition Dec 22, 2021

ciaranmcnulty Mar 12, 2022

loic425 Mar 13, 2022

loic425 commented Dec 22, 2021 •

edited

shouze commented Jan 30, 2022

webignition commented Jan 31, 2022

loic425 commented Jan 31, 2022 •

edited

loic425 commented Mar 7, 2022

loic425 commented Mar 8, 2022 •

edited

loic425 commented Mar 9, 2022 •

edited

ciaranmcnulty commented Mar 9, 2022

loic425 commented Mar 9, 2022

ciaranmcnulty commented Mar 11, 2022

ciaranmcnulty commented Mar 12, 2022 •

edited

loic425 commented Mar 13, 2022

ciaranmcnulty commented Mar 13, 2022

loic425 commented Mar 13, 2022

ciaranmcnulty commented Mar 13, 2022

loic425 commented Mar 14, 2022

Symfony 6 support (without Flex dependency) #1402

Symfony 6 support (without Flex dependency) #1402

Conversation

loic425 commented Dec 1, 2021

ciaranmcnulty commented Dec 1, 2021

loic425 commented Dec 2, 2021

webignition commented Dec 7, 2021

loic425 commented Dec 7, 2021

loic425 commented Dec 15, 2021

Choose a reason for hiding this comment

webignition Dec 17, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

loic425 commented Dec 22, 2021 • edited

shouze commented Jan 30, 2022

webignition commented Jan 31, 2022

loic425 commented Jan 31, 2022 • edited

loic425 commented Mar 7, 2022

loic425 commented Mar 8, 2022 • edited

loic425 commented Mar 9, 2022 • edited

ciaranmcnulty commented Mar 9, 2022

loic425 commented Mar 9, 2022

ciaranmcnulty commented Mar 11, 2022

ciaranmcnulty commented Mar 12, 2022 • edited

loic425 commented Mar 13, 2022

ciaranmcnulty commented Mar 13, 2022

loic425 commented Mar 13, 2022

ciaranmcnulty commented Mar 13, 2022

loic425 commented Mar 14, 2022

webignition Dec 17, 2021 •

edited

loic425 commented Dec 22, 2021 •

edited

loic425 commented Jan 31, 2022 •

edited

loic425 commented Mar 8, 2022 •

edited

loic425 commented Mar 9, 2022 •

edited

ciaranmcnulty commented Mar 12, 2022 •

edited