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

refactor: Migrate away from deprecated ILogger to PSR-3 #1674

Merged
merged 1 commit into from
Aug 19, 2024

Conversation

susnux
Copy link
Contributor

@susnux susnux commented Aug 19, 2024

Mostly replace ILogger with LoggerInterface and some minor cleanup (constructor property promotion).

Some places used the deprecated logException, this is easy to migrate: Simply use the appropriate log level on the logger and place the exception under the exception key in the context.

Mostly replace `ILogger` with `LoggerInterface` and some minor cleanup (constructor property promotion).

Some places used the deprecated `logException`, this is easy to migrate:
Simply use the appropriate log level on the logger and place the exception under the `exception` key in the context.

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
@@ -98,7 +97,7 @@ public function __construct(
IUserManager $userManager,
IRootFolder $rootFolder,
IL10N $l10n,
ILogger $logger,
mixed $logger, // unused, only kept for compatibility with server
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Damn this is ugly.
Server should use DI and the app should register the service if needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I know but there are also other parameters where this is already done 😔

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But server already checks class_exists, meaning it can be auto-loaded, so we should theoretically be able to simply DI in server.

@ArtificialOwl ArtificialOwl merged commit 610775b into master Aug 19, 2024
29 checks passed
@susnux susnux deleted the refactor/migrate-away-from-ILogger branch August 19, 2024 15:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants