-
Notifications
You must be signed in to change notification settings - Fork 185
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 return types for v3 #77
Conversation
0caaede
to
3446186
Compare
@@ -27,7 +27,7 @@ interface LoggerInterface | |||
* | |||
* @return void | |||
*/ | |||
public function emergency(string|\Stringable $message, array $context = []); | |||
public function emergency(string|\Stringable $message, array $context = []): void; |
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.
Hi @Crell,
It would be way more useful to apply basic types and return types in let's say v2 to cover our needs in php 7.4.
As you mention Stringable interface is introduce in php 8, so we with v7.4 are out of luck to use types where we could:
- return type
- ?LoggerInterface $logger = null; in LoggerAwareTrait.
If you can publish v2 for php7.x without Stringable it would be awesome,
and keep v3 for php8.
Thanks!
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.
A child class is allowed to widen argument types (on PHP 7.2+, you can remove it, and on PHP 7.4+ you have full variance rules). So you can implement your method as public function emergency($message, array $context = []): void
and having your implementation compatible with interfaces v1, v2 and v3 on PHP 7.2+
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 did similar thing. But isn't the goal of this to abstract/extract/unify implementations so we don't have to do it one by one?
The version selector could do that for us, as said if you publish v2.x as PHP7.4 compatible, just without Stringable.
But anyway, thanks for the response.
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.
@vukanac implementations are either expected to implement exactly the PSR interface of a single supported version of psr/log or to be nice to their user regarding dependency conflicts by relying on PHP variance rules (available in PHP 7.2+) to write an implementation compatible with multiple versions of psr/log. There is no reason for psr/log to publish another version of the interface supporting PHP 7.x (which cannot be 2.x as there is already a published 2.0 signature and changing it would be a BC break). If you want to keep compatibility with PHP 7.4, keep support for psr/log 1
The return types are all void, so this is a pretty boring addition. This necessarily includes the v2 branch, which should be reviewed in its own PR #76.
See the one extra commit for the specific changes in this PR.