Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.6k
[MonologBridge] Fix support for monolog 3.0#52222
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
7553021 to107428fCompareCan you add a test to prevent future regressions? |
I think it's difficult in this PR to add test because mock stream functions will cause a refactoring of the command. |
8a05567 to3555d5aCompareI don't know how to fix psalm error because in monolog 2.x, LogRecord is an interface and psalm runs with this version. |
| channel: $record['channel'], | ||
| context: $record['context']->getContext(), | ||
| datetime: $record['datetime'], | ||
| message: $record['message'], |
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.
let's not use named arguments if possible
louismariegaboritOct 29, 2023 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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.
Ok I did the change. But why don't use named arguments ?
Named arguments are already used for the LogRecord.
| message: (string)$message, |
Don't care about Psalm in this case. This is an expected error due to supporting several versions of Monolog. Our psalm setup only reportsnew issues for each PR, so this won't both future PRs. |
e5afa36 to3aacc28CompareGood catch, thanks@louismariegaborit. |
…ouismariegaborit)This PR was merged into the 6.3 branch.Discussion----------[MonologBridge] Fix context data and display extra data| Q | A| ------------- | ---| Branch? | 6.3| Bug fix? | yes| New feature? | no| Deprecations? | no| Issues | ->| License | MITFix the compatibility with Monolog 3. In the PR#52222 we had the compatibility with Monolog 3 but context parameter is incorrect and extra parameter is missing.No test can be added because testing this command is not possible at this time. Another PR (#53518) was opened to add test on this command.Commits-------8687ae0 Fix context data and display extra data
Uh oh!
There was an error while loading.Please reload this page.
handlefunction in HandlerInterface supports only LogRecord argument in monolog 3.0.