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

Drop PHP support <8.0 #2131

Merged
merged 3 commits into from
Dec 7, 2022
Merged

Drop PHP support <8.0 #2131

merged 3 commits into from
Dec 7, 2022

Conversation

sidz
Copy link
Contributor

@sidz sidz commented Dec 5, 2022

This PR drops support for PHP <8.0 as discussed in #1961

Copy link
Owner

@ruflin ruflin left a comment

Choose a reason for hiding this comment

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

I'm curious how much push back we will get on this as we are basically only 8 days after the EOL of 7.4. But I think it is the right decision. If we have many users with lots of issues, it is easier to add it later again as removing is the breaking change.

Screenshot 2022-12-06 at 09 26 33

Does anyone know if there are some numbers out there on how well users migrated to 8?

@sidz
Copy link
Contributor Author

sidz commented Dec 6, 2022

@ruflin I suppose we could rely on packagist statistics: https://packagist.org/php-statistics.
There are more users with PHP 8.1 rather than 8.0 . Migration from 8.0 to 8.1 is smth like from 7.0 to 7.1

Comment on lines 43 to 47
include:
# Test with the lowest set of dependencies
- dependencies: 'lowest'
php: '7.2'
elasticsearch: '7.15.2'
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should be modified instead of removed, the idea of this job is to run the tests with the lowest set of dependencies.

Let's say we require elasticsearch/elasticsearch: ^8.0 package, this job would use 8.0 to run the tests, otherwise we would only test with the latest release (right now 8.5.2)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah the idea is clear but currently (in this PR) we don't have lower dependencies (except probably psr/log). We could reintroduce testing against lower dependencies soon when elasticsearch/elasticsearch will be updated and we decide which minimum version of it we will use

Copy link
Owner

Choose a reason for hiding this comment

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

Maybe a compromise here is to comment it out for now and leave a comment. I'm a bit worried we might forget to readd it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

reverted

Copy link
Owner

Choose a reason for hiding this comment

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

thx

@ruflin
Copy link
Owner

ruflin commented Dec 6, 2022

Oh, 7.4 is still at around 28% but good to see it is decreasing ... 8.* is about 60% which is promising.

@ruflin
Copy link
Owner

ruflin commented Dec 7, 2022

@franmomu Can you have another look?

@ruflin
Copy link
Owner

ruflin commented Dec 7, 2022

@sidz One thing I only realised now: Changelog is missing. I would argue this is worth to be mentioned :-D

@deguif We should also update the README page: https://github.com/ruflin/Elastica

@sidz
Copy link
Contributor Author

sidz commented Dec 7, 2022

@ruflin Changelog amended

@ruflin ruflin merged commit 512f731 into ruflin:8.x Dec 7, 2022
@ruflin
Copy link
Owner

ruflin commented Dec 7, 2022

Thanks @sidz

@sidz sidz deleted the update-php-with-dependecies branch December 7, 2022 09:26
@sidz sidz mentioned this pull request Dec 7, 2022
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.

4 participants