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

Add explicit nullable types for PHP 8.4 support #162

Conversation

SanderMuller
Copy link
Contributor

@SanderMuller SanderMuller commented Nov 14, 2024

This update adds explicit nullable types to method signatures to ensure compatibility with PHP 8.4, as implicit nullability is deprecated.

  • Updated method signatures across multiple classes to use explicit nullable types (?Type).
  • Modified the buildCacheKey method in CacheRepository and related methods in drivers and interfaces.
  • Started work on the upgrade guide and changelog for PHP 8.4 compatibility.
  • Updated workflows to align with the new PHP version requirements.

Consideration:

Since this will require a major version due to the signature changes, perhaps this is a good moment to drop unsupported PHP versions 8.0 and Laravel 8 and 9.

@ash-jc-allen
Copy link
Owner

Woah! Huge thank you for all the effort you've put into this @SanderMuller, I really appreciate it! 🤯

I'm hoping to have a proper sit down and go through all these changes tomorrow or over the weekend 😄

@SanderMuller
Copy link
Contributor Author

Woah! Huge thank you for all the effort you've put into this @SanderMuller, I really appreciate it! 🤯

I'm hoping to have a proper sit down and go through all these changes tomorrow or over the weekend 😄

Let me know if I can help with this process! I think the impact is quite minimal

@ash-jc-allen
Copy link
Owner

Hey @SanderMuller! I'm really sorry for not getting to this yet. It's been a couple of hectic weeks 😭 I've set some time aside this weekend to go over all my packages and get them updated for PHP 8.4. So I'll make sure I take a look at this then! 😄

@SanderMuller
Copy link
Contributor Author

Hey @SanderMuller! I'm really sorry for not getting to this yet. It's been a couple of hectic weeks 😭 I've set some time aside this weekend to go over all my packages and get them updated for PHP 8.4. So I'll make sure I take a look at this then! 😄

Hope you're doing well, take your time!

@ash-jc-allen ash-jc-allen merged commit b3fc39d into ash-jc-allen:master Dec 1, 2024
29 checks passed
@ash-jc-allen
Copy link
Owner

Hey @SanderMuller! I'm just letting you know I've released these changes in a v7.7.0 release! 🎉

https://github.com/ash-jc-allen/laravel-exchange-rates/releases/tag/v7.7.0

I opted for a minor version instead because it seems like the signatures are pretty much the same. From what I can see when reading up, it appears that optional parameters were implicitly nullable, and adding the ? just make explicitly makes them nullable without changing how they function. I'm happy to be corrected on that if I'm wrong though!

I also looked at how Laravel handled it too, and it appears they just did a minor version bump (laravel/framework#50922).

Huge thanks again for this PR, I appreciate it! 😄

@SanderMuller
Copy link
Contributor Author

Hey @SanderMuller! I'm just letting you know I've released these changes in a v7.7.0 release! 🎉

https://github.com/ash-jc-allen/laravel-exchange-rates/releases/tag/v7.7.0

I opted for a minor version instead because it seems like the signatures are pretty much the same. From what I can see when reading up, it appears that optional parameters were implicitly nullable, and adding the ? just make explicitly makes them nullable without changing how they function. I'm happy to be corrected on that if I'm wrong though!

I also looked at how Laravel handled it too, and it appears they just did a minor version bump (laravel/framework#50922).

Huge thanks again for this PR, I appreciate it! 😄

I think that's fine, at least for our project! Maybe if someone extended these classes, they are forced to update their signature (or can you extend ?string $val = null with string $val = null?, either way that feels quite minor and only seconds of work.

Thanks for the tag!

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.

2 participants