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
[PHP 8.4] Fixes for implicit nullability deprecation #717
base: master
Are you sure you want to change the base?
Conversation
Fixes all issues that emits a deprecation notice on PHP 8.4. See: - [RFC](https://wiki.php.net/rfc/deprecate-implicitly-nullable-types) - [PHP 8.4: Implicitly nullable parameter declarations deprecated](https://php.watch/versions/8.4/implicitly-marking-parameter-type-nullable-deprecated)
Thanks for the contribution @Ayesh. We are currently hard at wok in reviving the repository and are currently focussing on cleaning up any old issues and pull request. After which we are going to work on merging PR's. This PR would work and resolve issues for PHP 8.4 without a doubt. But this repository also supports older version of PHP (currently still even PHP 5.3) which don't support the But still very much appreciated to put in the effort and bringing this to our attention. |
Thank you for the quick response @DannyvdSluijs. I was pleasantly surprised to see this library supporting PHP versions that old. I jumped to submit the fix when I saw the notices when running Composer, but you are right, it probably needs coordination to release a version with a higher minimum PHP version. I'm not sure if closing this PR will help. If that's the case, I will happily submit a new PR against the new branches if we are about to release a PHP 7.1+ version. Thank you. |
It would be nice to move quickly on this one so that the rest of us can enjoy a deprecation-free composer experience ASAP 🙏 |
Yeah this is a tricky one as it will require the php version bump, it is gonna have to be for v6, which does not exist yet, and there are quite a few open things before it could be tagged https://github.com/justinrainbow/json-schema/projects/2#card-11979917 But IMO this could be merged as soon as the php requirement is bumped, and perhaps an alpha version tagged at some point so we could use that in composer. |
In theory, this PR can be implemented using traits for all supported PHP versions. 35 changes, so max. 35 traits :) |
We already have problems finding enough time to maintain this lib, so let's please avoid suggesting ways to waste even more time :) |
This would require bumping the PHP requirement to at least PHP 7.1+, which can be made compatible with the composer needs of supporting PHP 7.2. |
What about releasing v5.3.0 with the bump to 7.1? |
Absolutely not. Increasing the PHP minimum version that far is a breaking change, and therefore necessitates a major version change for this library. We can't just drop it into the 5.x.x branch and not care about what we break. It could go in the v6 branch though (although please see this comment in relation to that). |
If the increase is done without forgetting to update the composer metadata, it won't break BC for projects installing it through composer as composer takes the PHP requirement into account when resolving dependencies (unlike what npm does with the node compatibility info). |
Yeah I am also a proponent of just bumping the php requirement in minor semver version bumps, which I think is completely fine as people can still rely on the old versions with older php versions, and it won't just update to the new one if they aren't on a modern enough php so this shouldn't break anything. From what I understood tho @erayd wrote that support for 5.3+ is still desirable for master/v6. So I don't know, what I would personally do but that's just my 2c:
I think this could all be done without much work and I'm happy to even do it but I would need @erayd's blessing :) |
it affects Drupal, hope it will be sorted out soon
|
@erayd any opinion on my post above? #717 (comment) |
Fixes all issues that emits a deprecation notice on PHP 8.4.
See: