Skip to content

enh(log): exit log() earlier if no crashreport registered - #43529

Merged
skjnldsv merged 2 commits into
masterfrom
enh/noid/leave-log-early-if-no-crash-reporter
Feb 23, 2024
Merged

enh(log): exit log() earlier if no crashreport registered#43529
skjnldsv merged 2 commits into
masterfrom
enh/noid/leave-log-early-if-no-crash-reporter

Conversation

@ArtificialOwl

@ArtificialOwlArtificialOwl commented Feb 12, 2024

Copy link
Copy Markdown
Member

follow-up on the previous improvement from @juliushaertl : #35970

The idea is to avoid some formatting of the payload as soon as we know for sure that it will not be used

@ArtificialOwl
ArtificialOwl requested review from a team, icewind1991, juliusknorr, nfebe and sorbaugh and removed request for a teamFebruary 12, 2024 14:47
@ArtificialOwlArtificialOwl added the 3. to review Waiting for reviews label Feb 12, 2024
@ArtificialOwlArtificialOwl added this to the Nextcloud 29 milestone Feb 12, 2024
@ArtificialOwl
ArtificialOwlforce-pushed the enh/noid/leave-log-early-if-no-crash-reporter branch from bf68594 to 18a6b62CompareFebruary 12, 2024 18:17
@ArtificialOwl

Copy link
Copy Markdown
MemberAuthor

(failing test not related)

@ArtificialOwl
ArtificialOwlforce-pushed the enh/noid/leave-log-early-if-no-crash-reporter branch from 18a6b62 to b9e0530CompareFebruary 13, 2024 10:02

@icewind1991icewind1991 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.

This would break BeforeMessageLoggedEvent

Signed-off-by: Maxence Lange <maxence@artificial-owl.com>
@ArtificialOwl
ArtificialOwlforce-pushed the enh/noid/leave-log-early-if-no-crash-reporter branch from b9e0530 to 5aff876CompareFebruary 14, 2024 10:41
@ArtificialOwl

Copy link
Copy Markdown
MemberAuthor

This would break BeforeMessageLoggedEvent

I added a hasListeners() and exit the method if no BeforeMessageLoggedEvent is registered.
Not sure the performance gain anymore.
Your thoughts ?

@ArtificialOwl
ArtificialOwlforce-pushed the enh/noid/leave-log-early-if-no-crash-reporter branch from 5aff876 to bf6cb92CompareFebruary 14, 2024 11:20
Comment threadlib/private/Log.php Outdated
Signed-off-by: Maxence Lange <maxence@artificial-owl.com>
@ArtificialOwl
ArtificialOwlforce-pushed the enh/noid/leave-log-early-if-no-crash-reporter branch from bf6cb92 to 2232753CompareFebruary 14, 2024 15:06

@icewind1991icewind1991 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.

log events still work

@skjnldsv
skjnldsv merged commit 5207274 into masterFeb 23, 2024
@skjnldsv
skjnldsv deleted the enh/noid/leave-log-early-if-no-crash-reporter branch February 23, 2024 20:56
@blizzzblizzz mentioned this pull request Mar 5, 2024
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to reviewWaiting for reviewsperformance 🚀

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@ArtificialOwl@icewind1991@skjnldsv