Skip to content

Conversation

abdelrahmenAyman
Copy link
Contributor

Change

Change helper function logger return type to ?LoggerInterface instead of ?LoggerManager. LoggerManager implements LoggerInterface, which means any code expecting a LogManager should still work fine since this is the wider type.

Motivation

The return type was introduced in a recent PR #56684 . Introducing the change to our codebase broke multiple tests where we mocked the logger function using a LoggerInterface mock instance.
Programming for interfaces is a widely accepted practice in the industry, thus the function should be returning an Interface instead of a concrete type to be flexible enough for any more logger classes in the future.

Copy link

Thanks for submitting a PR!

Note that draft PR's are not reviewed. If you would like a review, please mark your pull request as ready for review in the GitHub user interface.

Pull requests that are abandoned in draft may be closed due to inactivity.

@matthewdevine
Copy link

I'll just add my two cents and say we were having the same issue. Came from a separate library (LogFake), but this would resolve the issue I was having.

@abdelrahmenAyman abdelrahmenAyman force-pushed the fix-logger-helper-concrete-return-type branch from a4ac716 to 9973737 Compare September 21, 2025 14:07
@abdelrahmenAyman abdelrahmenAyman marked this pull request as ready for review September 21, 2025 14:14
@abdelrahmenAyman
Copy link
Contributor Author

@crynobone This should be ready now!

@taylorotwell taylorotwell merged commit 559976f into laravel:12.x Sep 21, 2025
63 checks passed
tegos pushed a commit to tegos/laravel-framework that referenced this pull request Sep 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs work Not quite ready for primetime
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants