Skip to content

[26] allow printing log messages during occ - #998

Merged
blizzz merged 2 commits into
stable26from
occ-log-printer-26
Nov 15, 2023
Merged

[26] allow printing log messages during occ#998
blizzz merged 2 commits into
stable26from
occ-log-printer-26

Conversation

@icewind1991

@icewind1991icewind1991 commented Sep 19, 2023

Copy link
Copy Markdown
Member

This was referenced Sep 20, 2023
@blizzzblizzz mentioned this pull request Oct 9, 2023
blizzz
blizzz previously requested changes Oct 10, 2023

@blizzzblizzz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

BeforeMessageLoggedEvent was introduced in 27.0.1 only

(that's what the require is about, just blocking to avoid accidental merge)

@blizzz

Copy link
Copy Markdown
Member

CI is failing

ERROR: InvalidArgument - lib/Listener/LogListener.php:58:62 - Argument 3 of OCA\LogReader\Log\Console::log expects array<array-key, mixed>, but string provided (see https://psalm.dev/004)
$this->console->log($event->getLevel(), $event->getApp(), $event->getMessage());

I would have expected this on the master and 27 PRs as well, but there it didn't appear.

@susnux

susnux commented Nov 9, 2023

Copy link
Copy Markdown
Contributor

@blizzz on master & stable27 the typing of BeforeMessageLoggedEvent is correct, but for stable26 the typing of getMessage() is string instead of array.

@susnux

Copy link
Copy Markdown
Contributor

@blizz

This comment was marked as resolved.

@susnux

This comment was marked as off-topic.

@blizzzblizzz mentioned this pull request Nov 13, 2023
Signed-off-by: Robin Appelman <robin@icewind.nl>
Signed-off-by: Robin Appelman <robin@icewind.nl>
@blizzz
blizzz merged commit 20d69a6 into stable26Nov 15, 2023
@blizzz
blizzz deleted the occ-log-printer-26 branch November 15, 2023 20:58
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@icewind1991@blizzz@susnux@blizz@come-nc