search

JayBizzle / Crawler-Detect

🕷 CrawlerDetect is a PHP class for detecting bots/crawlers/spiders via the user agent
https://crawlerdetect.io
MIT License
2.01k stars 258 forks source link

Compatability with php 8.4 #543

Closed srjlewis closed 3 days ago

srjlewis commented 1 month ago

Deprecate implicitly nullable parameter types https://wiki.php.net/rfc/deprecate-implicitly-nullable-types

srjlewis commented 1 month ago

Hi

I have changed composer and git workflow to support PHP 7.1 and above, since nullable types cant be use in older versions of PHP.

acelaya commented 3 weeks ago

PHP 8.4 will be released in a couple of weeks. Can we get this merged?

If dropping support PHP older than 7.1 is a problem, another possible approach is to simply remove the native typehint entirely, and document the types via PHPDoc.

JayBizzle commented 3 weeks ago

Trying to decide if i should release this as a v2.0 which i think we will have to do if we are dropping support for older PHP versions.

Old PHP version installs are negligible these days - https://packagist.org/packages/jaybizzle/crawler-detect/php-stats

Anyone got any thoughts on this?

acelaya commented 3 weeks ago

Bumping dependency requirements is usually not considered a breaking change. If v1.3 was to be released requiring PHP 7.1, anyone depending on ^1.0 and using an older PHP version, would still get 1.2.x and everything would continue working for them (assuming they use composer, but in almost 2025, I don't think anything else needs to be even considered).

On the other hand, if you want to continue providing patches for older PHP versions, bumping the major to 2 will ease your work in backporting patches between versions.

So it depends on what you want to do and promote.

As a fellow open source maintainer, I would recommend you make your life easier and simply drop support for older versions. PHP 7.1 is 8 years old, people have had time to update.

As a consumer of this library, I don't have an opinion. Both options work for me and will be as easy to update.

JayBizzle commented 3 weeks ago

Thanks for your inout @acelaya

I think we will go down the v1.3 route 👍

acelaya commented 1 week ago

Is there anything else that is preventing this to be merged?

JayBizzle commented 1 week ago

Is there anything else that is preventing this to be merged?

Just time. Ill try and get it sorted this week 🙏

acelaya commented 5 days ago

In case anyone else is still a bit blocked by this, I have just published a fork which includes these changes: acelaya/crawler-detect.

I have not marked it as a replacement of jaybizzle/crawler-detect, as I plan to mark it as abandoned as this is merged and released here.

JayBizzle commented 3 days ago

Finally released v1.3.0

Sorry for the delay.