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

Update phpunit.yml with PHP 8.1 and 8.2 #616

Merged
merged 2 commits into from
Jul 26, 2023
Merged

Conversation

casperbakker
Copy link
Member

No description provided.

@casperbakker casperbakker merged commit 97cd265 into main Jul 26, 2023
4 checks passed
@casperbakker casperbakker deleted the casperbakker-patch-1 branch July 26, 2023 06:19
@DannyvdSluijs
Copy link
Contributor

Out of curiosity do these changes mean support for PHP 7.2 and PHP 7.3 are being dropped?

@casperbakker
Copy link
Member Author

@DannyvdSluijs I do very little with this package, so it is not up to me. I think @stephangroen, @remkobrenters and probably you can better decide when to drop support for older versions.

Stephan is out of the office right now and I saw a PR and some small things I could easily tweak. That is why I added 8.1 and 8.2 to test against. And I think it is a waste of energy to run to many tests, so I removed the oldest versions.

If we want to drop support for PHP 7, we should update the composer.json file. I am all for that, because I think that people who run unsupported PHP versions likely also don't update their composer dependencies. But not my intention right now to drop support.

@remkobrenters
Copy link
Collaborator

I would suggest to allow legacy versions to be supported as long as they are not making things more complicated to maintain and do not block the implementation of new features.

@casperbakker
Copy link
Member Author

That is also my general take. But if there are some nice new features that are available in the oldest supported PHP version, then I am usually pretty easy with dropping old versions. That is what I do for packages that I am the main maintainer of, like https://github.com/picqer/php-barcode-generator.

It is nice that it will not break older installations. Composer will just not update that installation further then the latest supported version. That makes it easier to drop support for versions that are already unsupported for 1-2 years.

Versions 7.4 and 8.0 have some nice new features that can make this library better to read. So dropping all 7.x versions can make the interface better.

@remkobrenters
Copy link
Collaborator

Agree. @stephangroen as endboss of the project, what is your view on this?

@stephangroen
Copy link
Member

It's nice to keep allowing for legacy versions for a reasonable amount of time, but I'm a proponent of staying near the current versions. I'd hate for this package to be kept back because we support legacy versions for too long. It blocks progression. I get that it's not always easy with client work to be forced to upgrade though.

PHP 7.x is already EOL for quite some time. PHP 8.0 is already on security fixes only.

I'm for dropping support for PHP < 7.4. PHP 7.4 is then the latest legacy version we support for now. I'll update composer.json before merging.

I'll wait a bit on input by @remkobrenters and long time contributor @DannyvdSluijs before merging this.

@remkobrenters
Copy link
Collaborator

Yeah agree. Keep support as long it is not in the way and when it gets in the way just drop support.

@DannyvdSluijs
Copy link
Contributor

DannyvdSluijs commented Aug 22, 2023

In not having any strong feelings for keeping php 7.x support around. But I can imagine that it is helpful for some to still have 7.4 support in this package. Looking at the install statistics on packagist there install percentage of < 7.4 seems 3%.

My questioning was purely to see if we can improve on type hints and other new constructs that are part of newer language versions. This would improve the experience for the 97% that are on newer versions. Which a can follow up after the proposed changes from Stephan.

@stephangroen
Copy link
Member

Released V4 with dropped 7.2 and 7.3 support.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants