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

Fix: Drop support for PHP 7.4 #733

Closed
wants to merge 1 commit into from

Conversation

localheinz
Copy link
Member

@localheinz localheinz commented Sep 5, 2023

What is the reason for this PR?

  • drops support for PHP 7.4

πŸ’β€β™‚οΈ PHP 7.4 reached its end of life on November 28, 2022.

Author's checklist

Summary of changes

Review checklist

  • All checks have passed
  • Changes are approved by maintainer

@localheinz localheinz added the bug Something isn't working label Sep 5, 2023
@localheinz localheinz self-assigned this Sep 5, 2023
@localheinz localheinz force-pushed the fix/php7.4 branch 2 times, most recently from c212560 to b31bce9 Compare September 5, 2023 08:54
Copy link
Member

@GrahamCampbell GrahamCampbell left a comment

Choose a reason for hiding this comment

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

I would like to retain PHP 7 support, to encourage people to migrate from the old package to this one. I also feel like this package should retain a wide range of supported PHP versions.

@GrahamCampbell
Copy link
Member

We already had this conversation before. Half of people installing the old package last month were using PHP 7, and still more than 10% on our new package. I know people can use an older version of this package, but often people are not very competent at choosing package versions, and I don't want to put up blockers.

@localheinz localheinz reopened this Sep 5, 2023
@localheinz
Copy link
Member Author

Please allow maintainers, contributors, and users to discuss proposed changes before closing pull requests you disagree with, @GrahamCampbell.

@GrahamCampbell
Copy link
Member

I oppose this change in the strongest possible terms. It's so important that testing frameworks retain a large range of supported PHP versions. We already compromised by dropping PHP 7.2 and 7.3. If we drop 7.4 too, and PHP 8.3 or 8.4 make breaking changes, and we add new features, it may be impossible to test code on both PHP 7.4 and 8.3/8.4 without a mess to support two different versions of faker. We've already seen how much a mess this was with PHPUnit - Symfony's simple phpunit should not have been necessary.

@GrahamCampbell GrahamCampbell removed the bug Something isn't working label Sep 5, 2023
@bram-pkg
Copy link
Member

bram-pkg commented Sep 5, 2023

Feel like I have to agree with Graham here, it's good for dev dependencies to have a less strict restriction on PHP versions. Looking at the data; a not insignificant amount of downloads still come from <8.0, so as long as it's not a maintenance issue for us to support 7.4, let's keep it around.

That being said, let's indeed not close pull requests or issues without chance for a discussion.

@localheinz
Copy link
Member Author

@GrahamCampbell

We already had this conversation before. Half of people installing the old package last month were using PHP 7, and still more than 10% on our new package. I know people can use an older version of this package, but often people are not very competent at choosing package versions, and I don't want to put up blockers.

fzaninotto/faker only supports PHP 5 and 7, so it's only natural that there is a significant percentage of people who still use fzaninotto/faker on PHP 7.

Reasons for that percentage to go up

  • people who use fzaninotto/faker on PHP 5 migrate to PHP 7
  • people who use fzaninotto/faker on PHP 5 migrate away from fzaninotto/faker

Reasons for that percentage to go down

  • people who use fzaninotto/faker on PHP 7 migrate away from fzaninotto/faker

I oppose this change in the strongest possible terms. It's so important that testing frameworks retain a large range of supported PHP versions. We already compromised by dropping PHP 7.2 and 7.3. If we drop 7.4 too, and PHP 8.3 or 8.4 make breaking changes, and we add new features, it may be impossible to test code on both PHP 7.4 and 8.3/8.4 without a mess to support two different versions of faker. We've already seen how much a mess this was with PHPUnit - Symfony's simple phpunit should not have been necessary.

If we drop support for PHP 7.4, we do not need to run tests for fakerphp/faker on PHP 7.4. We also do not need to support multiple versions of fakerphp/faker.

By dropping support for PHP 7.4, we - like everyone else who drops support for outdated PHP versions from their PHP packages, increase the pressure on application maintainers to update from outdated to supported PHP versions.

This is a good thing.

@bram-pkg

Feel like I have to agree with Graham here, it's good for dev dependencies to have a less strict restriction on PHP versions. Looking at the data; a not insignificant amount of downloads still come from <8.0, so as long as it's not a maintenance issue for us to support 7.4, let's keep it around.

These people can still install older versions of fakerphp/faker, and as I suggested above, by dropping support for outdated PHP versions from fzaninotto/faker, we increase the pressure on application and package maintainers to update from outdated to supported PHP versions and drop support for unsupported PHP versions.

Generally

It probably makes sense to adopt a general PHP version support policy for this package - for how long after the end of life of a PHP version do we want to support that PHP version?

@deleugpn
Copy link

deleugpn commented Sep 5, 2023

I'm with Graham here. I'm all in favor of maintainers pushing the ecosystem forward by reducing PHP supported version in their libraries if they want to. Testing library, however, is a major factor in version upgrades. We need to upgrade our codebase and trust our tests will catch breaking changes or issues that would otherwise become bugs in production. If production code is being touched, it can be desired to avoid modifying tests. If test code is being touched, it can be desired to avoid modifying production code. That increases confidence and helps with upgrades.

Unless there's a relevant benefit for the package itself to upgrade, I'm inclined to think that forcing PHP upgrades on testing libraries causes more harm than good in the goal of getting projects to upgrade.

@localheinz
Copy link
Member Author

@deleugpn

The current version of fakerphp/faker continues to work even when we drop support for PHP 7.4 here, doesn't it?

@pimjansen
Copy link

Im not a favor of the drop though i would like to.

It brings us not a single benefit dropping older versions and instead of breaking the 1.x branch i'd rather work om finalizing 2.x.

If im correct the main topic there is migrating the existing locale and then we can even publish a first alfa already

@localheinz
Copy link
Member Author

@bram-pkg @deleugpn @GrahamCampbell @pimjansen

Can we agree to drop support for PHP 7.4 in 2.x?

@pimjansen
Copy link

I think if we have to change to type stuff we should yes. And from there on we can properly maintain it when we do not drag all the code ourself and a bunch of legacy

@deleugpn
Copy link

deleugpn commented Sep 6, 2023

@bram-pkg @deleugpn @GrahamCampbell @pimjansen

Can we agree to drop support for PHP 7.4 in 2.x?

@localheinz To me, a feature-complete library like Faker only needs code change to make itself compatible with future versions of PHP i.e. addressing breaking changes or syntax changes.

Also, I like what Symfony does with their new major version which is an exact copy of the previous version minus deprecations-only. Tagging a 2.0 version where PHP 7.4 is no longer supported causes dependency-creep where updating a dependency forces the update of a PHP "Era" version (7.4 to 8.0). If PHP support needs to be dropped, perhaps allowing dependencies to gracefully handle package update first (2.0 with >=7.4) and PHP update second (2.1 with >=8.0) would make upgrade easier.

Overall, dropping support for a PHP version just for the sake of dropping it on a core component of test suites across the PHP community will, in my opinion, cause more upgrade distress than help the community keep up-to-date, which sounds to me the opposite of what you would like to accomplish.

@localheinz
Copy link
Member Author

@deleugpn

@localheinz To me, a feature-complete library like Faker only needs code change to make itself compatible with future versions of PHP i.e. addressing breaking changes or syntax changes.

That's an opinion, nothing more.

We have dropped support for PHP 5.4, PHP 5.5, PHP 5.6, PHP 7.0, PHP 7.1, PHP 7.2, and PHP 7.3. Where were your complaints for dropping support for these PHP versions back then? Where were the complaints of users who were put under distress then?

Also, I like what Symfony does with their new major version which is an exact copy of the previous version minus deprecations-only. Tagging a 2.0 version where PHP 7.4 is no longer supported causes dependency-creep where updating a dependency forces the update of a PHP "Era" version (7.4 to 8.0). If PHP support needs to be dropped, perhaps allowing dependencies to gracefully handle package update first (2.0 with >=7.4) and PHP update second (2.1 with >=8.0) would make upgrade easier.

Symfony is an open-source project. Unlike fakerphp/faker, Symfony is backed by a company and thousands of contributors. Core maintainers are paid by Symfony, the company. On top of that, core maintainers have sponsors. Members of the core team who work for companies like QOSSMIC (previously SensioLabs Deutschland) are getting compensated directly or indirectly by these companies to contribute to Symfony.

Symfony is also a commercial project. They want to maximize the number of users who use and lock in into the Symfony eco system.

I maintain and contribute to this project (and many others) in my spare time. I have five sponsors.

CleanShot 2023-09-06 at 07 52 41@2x

Symfony and fakerphp/faker are not the same.

Overall, dropping support for a PHP version just for the sake of dropping it on a core component of test suites across the PHP community will, in my opinion, cause more upgrade distress than help the community keep up-to-date, which sounds to me the opposite of what you would like to accomplish.

That's an opinion, nothing more. Where is your data?

The maintainer of the applications and packages who you believe will be put in distress are outsourcing their distress to the maintainers of the packages that you believe will put them in distress when they drop support for PHP versions that have reached their end of life.

As pointed out before on Twitter, there is no reason to be running PHP versions that have reached their end of life in production today, and consequently, there is no reason to support PHP versions that have reached their end of life as a package maintainer unless your goal is to maximize the number of users who use that package.

Existing users who are unable or unwilling to upgrade to PHP 7.4 can continue using current or older versions of this package. As @GrahamCampbell pointed out, they are doing it.

@deleugpn
Copy link

deleugpn commented Sep 6, 2023

@localheinz hey man, I'm taking myself off this discussion and unsubscribing. You shared this thread on Twitter and asked for people's opinion. I came here to give you mine and now you're just escalating things. I know everything I said is my opinion. That's what I thought I was doing, sharing my opinion. But apparently you're not looking for people to give their opinion, you're looking for validation and support for your decision. I'm sorry I can't give you that.

I'm running PHP 8.2 on all my production applications. If this library drops support for PHP 7.4, 8.0 and 8.1 today I won't lose one second of my life worrying about it.

Good luck to everyone involved. I hope the decision that makes the best impact on the PHP community is made, regardless of what that decision is.

@localheinz
Copy link
Member Author

localheinz commented Sep 6, 2023

@deleugpn

@localheinz hey man, I'm taking myself off this discussion and unsubscribing. You shared this thread on Twitter and asked for people's opinion. I came here to give you mine and now you're just escalating things. I know everything I said is my opinion. That's what I thought I was doing, sharing my opinion. But apparently you're not looking for people to give their opinion, you're looking for validation and support for your decision. I'm sorry I can't give you that.

Im not looking for opinions. I am looking for arguments for or against dropping support for PHP 7.4.

@localheinz
Copy link
Member Author

localheinz commented Sep 6, 2023

Closing in favour of focussing on 2.0.0.

Also see #747.

@localheinz localheinz closed this Sep 6, 2023
@localheinz localheinz deleted the fix/php7.4 branch September 6, 2023 10:43
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.

None yet

5 participants