Php84 #563
Php84 #563
Conversation
Removes PHP 8.4 depreciation warnings and just good style
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
|
PHP 8.4 has been released. |
|
Bump, lets get this merged. |
|
Is there anything I need to do to get this merged? Thanks. |
|
Maybe sign their Contributor License Agreement (CLA) if you did not do it already? |
|
Catching up, would this be backwards incompatible with earlier PHP versions? If so, this should include updates in the |
|
Not the author, but this appears to add typed args that have been supported since probably at least 7.4. The package version is set to >= 8. So this seems safe, but I'd say this package should also consider adding a minimum and maximum php version to its test actions: That would quickly fail the PR. |
It should be backwards compatible. Changes such as |
Good catch. Are those changes actually needed? |
|
Original author here. I signed the agreement. Nullable types have existed since 7.1. Type parameters since 7.0. Return types since 7.1. Typed properties since 7.4. This package also only support PHP 8.0 and higher, so it should use all these features by default. We can't keep catering to people who refuse to update PHP on a timely basis. While typed parameters may break an extended class, you really should update your class. Also people not wanting to upgrade can still use an older version. Will ping someone I know at Google to see if we can move this forward. |
Yes, but, you know full well many of us have to work in broken ass organizations. |
|
This entire package probably needs a true maintainer. It doesn't even follow semver, its 1.3 release bumped to php8... Shouldn't that have been a major version bump as it dropped support for 7.x? |
|
I 100% support you adding these typed parameters! However, such breaking changes should go in a new major version, not in a minor/patch update which should be backwards compatible.
This attitude has caused so many problems over the years and is one of the reasons semver.org had to be created. Can we please not go back to the dark days of updates? People need to be able to trust software to work like it says it does.
Yes. Agreed. But that update changed requirements in composer.json so nobody with PHP < 8 would have gotten a nasty surprise. |
|
Then, What has to be done to have this PR accepted? PR adding support for PHP 8 (#517) was merged on February 2023, which is more than two years after the release of PHP 8.0 |
I would vote to split this PR into two:
|
|
I am happy to do what ever the maintainer of this repo wants. We just need a response. |
|
@rowan-m can you eventually check on this as you seem to have merging capabilities? |
|
Related, the release for PHP 8.4 compatibility in the enterprise repo: https://github.com/googleapis/google-cloud-php-recaptcha-enterprise/releases/tag/v1.17.2 |
|
the problem is that for my use case, this library is just used by a library I am using (https://github.com/karser/KarserRecaptcha3Bundle), so I would avoid having to use this because I should rewrite the symfony extension, or ask the dev to do it |
|
My contact at Google says there is apparently no group responsible for public repos. I am thinking to just publish my fork on Packagist. If @rowan-m or someone else decides to merge and stamp this PR, then I could just point it back to this repo. Unfortunately I have seen this from multiple companies with "Open Source" packages. They just don't seem to care about merging PR requests to keep the package up to date with the latest PHP releases. |
|
This should be left in the hands of a group we can trust. I wonder if someone at thephpleague (mentioning a random maintainer here @alcohol) or similar would assist with maintaining a fork. |
|
You can't really trust anyone. I don't for sure. That is why I check all code into git and do diffs on every composer update. Otherwise you open yourself up to supply chain attacks. Luckily PHP is all uncompressed text based, so easy to check. See my blog here for the solution. |
Any idea if they are willing/open to adding collaborators to public repositories? |
|
@rowan-m and all, I hope you're doing well! As we understand there are several contributors eager to collaborate on updating the package for compatibility with PHP 8.4. With PHP 8.4 having been GA for over a month and a half now, many consumers of the package are facing blockers when trying to use it with earlier PHP versions. We believe that updating the package to be fully compatible with PHP 8.4, while also addressing potential backwards incompatibilities with previous PHP versions, would greatly benefit the community. Many contributors are ready to assist and work together to make this happen as soon as possible. Would you be able to provide any updates on the status of the PR or let us know if there's anything further we can do to help move this forward? Thank you so much for your time and consideration. We're looking forward to collaborating and helping make this update happen. |
|
Hi all, @phpfui could you go forward publishing your fork/PR to packagist? With no one responsible, this could go on for months. |
|
Will do. Should have something next week. I want to convert it to GitHub actions, as I seem to remember that needed work. |
|
OK, I had some time. This should be a drop in replacement for google/recaptcha https://github.com/phpfui/recaptcha phpfui/recaptcha Just replace google with phpfui in your composer.json file and you should be good to go. It is modernized library with full type checking so run PHPStan on your code again to double check things. |
Post on php subreddit to get some traction on a fork because there is strength in numbers imo. I am leary of trusting forks to be maintained, but I will probably move my code over to your fork. |
|
@phpfui it would be great if you open the "issues" section on your new repository in case someone needs to open one |
|
Thanks for the heads up on the Issues tab. Now fixed. I thought that was the default, but GitHub has been adding so many features recently, maybe the defaults changed. @cnizzardini I tried to post in the PHP subredit, but it was instantly blocked by moderators (auto blocked I suspect). I really dislike Redit for a variety of reasons. Can you post it there? This package is getting a lot of downloads daily, and that has to be from this repo and the comments, as it has not been promoted anywhere else. Also I am a very conscientious open source maintainer. I respond to issues and pull requests within days if I am not traveling. Check out the NeonXP/MathExecutor project, or my ConstantContact or ICalendar repos for examples. |
|
I suspect this package will be/has been abandoned in favour of cloud-recaptcha-enterprise. Certainly, Google has pushed to migrate to that with "secret key" now being renamed "legacy secret key". I'm sure at some point they will drop support for that too. Feels like Google are trying to overcomplicate as many of their services as possible at the moment (Custom Search, Maps, GA4 - I'm looking at you). |
|
I just received this email yesterday from Google Cloud: Migrate Google reCAPTCHA key to a Google Cloud Project by the end of 2025 So yes, @jamieburchell it looks like this library will go away. But for now, my PHP 8.4 port is gaining 100-200 installs a day, so people need a solution for PHP 8.4 now. |
Me too. That's what ultimately led me here. I have been migrating sites to Google Cloud Platform this morning so that Google doesn't take it upon itself to create a mess of new projects in my console.
Absolutely. There must be millions of websites and third party software still using the "classic reCAPTCHA" - mine included. It's good to know there's a PHP 8.4 fork I can use. |
|
Final word from me: https://www.cloudflare.com/application-services/products/turnstile/ |
I could not run the composer scripts on my Windows machine, but PHPUnit runs in the normal way. I did not run PHPCSFixer.
No logic changes were made.