-
Notifications
You must be signed in to change notification settings - Fork 2k
feat(cache): add native return types to all CacheInterface methods #9811
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
base: 4.7
Are you sure you want to change the base?
feat(cache): add native return types to all CacheInterface methods #9811
Conversation
8065d0f to
622dd8b
Compare
+ remove deprecated `false` type in `getMetaData()` method + remove unnecessary @inheritdoc annotation
+ clarify deprecation removal of `false` type for `CacheInterface::getMetaData()`
622dd8b to
ff35267
Compare
| * @param mixed $value | ||
| */ | ||
| public function save(string $key, $value, int $ttl = 60) | ||
| public function save(string $key, $value, int $ttl = 60): bool |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you can natively type the mixed param without breaking LSP, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch, the type is actually bool, so I removed the docs declaration.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, what I meant was you can write mixed $value in the method signature.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, my bad! I was too quick 😅
Yes, just tried it, you can natively type the mixed param without breaking LSP.
That being said, should I not just change the $value signature in the CacheInterface as well? (so it's explicit)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, pls replicate the change to other handlers and the interface itself.
fba94bc to
2df6524
Compare
Description
Following the previous PR (#9809), this is the second part of the CacheInterface refactor.
This PR adds native return types to all CacheInterface methods and updates the user guides accordingly.
Checklist: