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

Bump to PHP 7.2, stick to phpunit 8.5 #492

Merged
merged 1 commit into from
Sep 9, 2024
Merged

Bump to PHP 7.2, stick to phpunit 8.5 #492

merged 1 commit into from
Sep 9, 2024

Conversation

nicolas-grekas
Copy link
Member

I think it's start to shade PHP 7.1 out. Of course the polyfill will be kept functional, but we won't care about it anymore from a maintenance PoV.

Sticking to phpunit 8.5 so that we can focus on one version of it, and because it has lifetime support and PHP 7.2 as a minimum version.

@nicolas-grekas nicolas-grekas force-pushed the php72 branch 2 times, most recently from 9a4f5b9 to 6b33d14 Compare September 9, 2024 09:23
@derrabus
Copy link
Member

derrabus commented Sep 9, 2024

What has changed since #428?

From Symfony's PoV, taking into account that the active maintenance of the 5.4 branch will end about two months from now, we might as well wait a little longer and bump to PHP 8.1 directly.

@nicolas-grekas
Copy link
Member Author

nicolas-grekas commented Sep 9, 2024

I made this PR after realizing that #486 was required because on PHP 7.1 we have to use var-dumper 4.4, which didn't get the fixes for nullable arguments.
Then I also realized that phpunit 8.5 has lifetime support and also php >= 7.2.
The CI is behind at the moment with e.g. #425 + failures and I don't think it would be productive to work on this while also having to consider several versions of phpunit.

PHP 8.1 is too early IMHO, at least I don't see similar arguments.

@derrabus
Copy link
Member

derrabus commented Sep 9, 2024

If PHP 7.1 support really blocks our CI, go ahead then. But please, let's do one release of polyfill-php72 as a no-op package as I proposed in #492 (comment)

PHP 8.1 is too early IMHO, at least I don't see similar arguments.

Let's revisit that topic in December or January. 🙂

@nicolas-grekas
Copy link
Member Author

let's do one release of polyfill-php72 as a no-op package

why did we do this? what's the benefit?
people with php < 7.2 can still install a previous version

@derrabus
Copy link
Member

derrabus commented Sep 9, 2024

let's do one release of polyfill-php72 as a no-op package

why did we do this? what's the benefit?

If you're on PHP >= 7.2, but one of your dependency's dependencies requires the polyfill, you get a package with zero overhead. Once we tagged a new release this way, we can still remove the package from the monorepo.

@nicolas-grekas
Copy link
Member Author

Got it, thanks. PR updated.

@nicolas-grekas nicolas-grekas merged commit 2b1532e into 1.x Sep 9, 2024
12 of 18 checks passed
@nicolas-grekas nicolas-grekas deleted the php72 branch September 9, 2024 12:04
This was referenced Sep 9, 2024
nicolas-grekas added a commit that referenced this pull request Sep 9, 2024
This PR was merged into the 1.x branch.

Discussion
----------

Remove src/Php72

Follows #492 (comment)

We can stop subtree-splitting symfony/polyfill-php72

Commits
-------

c5eab85 Remove src/Php72
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.

2 participants