-
-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Migrate away from ILogger in encryption #39065
Conversation
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.
Psalm found more than 10 potential problems in the proposed changes. Check the Files changed tab for more details.
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.
Logger migration looks consistent to me, left inline comments about code comments and type hinting.
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 I did not mention all the hint improvements, I guess if you are feeling good you can amend the latest improvements to last commit.
Thanks!
@@ -64,85 +59,44 @@ class Encryption implements IEncryptionModule { | |||
/** @var string */ | |||
private $user; | |||
|
|||
/** @var array */ | |||
private $owner; | |||
private array $owner; |
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.
Why is it the only one getting a type?
7b515c1
to
b1a7d07
Compare
And modernize code a bit Signed-off-by: Côme Chilliet <[email protected]>
…tructors Signed-off-by: Côme Chilliet <[email protected]>
Signed-off-by: Côme Chilliet <[email protected]>
b1a7d07
to
3e176f5
Compare
Migrate away from ILogger in encryption and switch to constructor parameter promotion in concerned classes.
See #32127
Checklist