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

[stable26] emit an event when a message is logged #38815

Merged
merged 2 commits into from Oct 11, 2023

Conversation

backportbot-nextcloud[bot]
Copy link

Backport of #38490

@backportbot-nextcloud backportbot-nextcloud bot added this to the Nextcloud 26.0.3 milestone Jun 14, 2023
@blizzz blizzz mentioned this pull request Jun 14, 2023
@blizzz blizzz added the 3. to review Waiting for reviews label Jun 14, 2023
blizzz
blizzz previously approved these changes Jun 14, 2023
@ChristophWurst
Copy link
Member

This is a backport of a new API. Is this really necessary or could there be a workaround?

@ChristophWurst
Copy link
Member

It's for nextcloud/logreader#873 so I would say this should not be backported.

@blizzz
Copy link
Member

blizzz commented Jun 15, 2023

This is a backport of a new API. Is this really necessary or could there be a workaround?

right, and since annotations also would need to be adjusted if we have a reason to backport, missed them.

/**
* Even for when a log item is being logged
*
* @since 28.0.0
Copy link
Member

Choose a reason for hiding this comment

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

change @SInCE annotations to specific 26.0.x target version.

@blizzz
Copy link
Member

blizzz commented Jun 15, 2023

moving to 26.0.4 then

@ChristophWurst ChristophWurst added the pending documentation This pull request needs an associated documentation update label Jun 15, 2023
@nextcloud nextcloud deleted a comment from hopklaas35 Jun 15, 2023
@blizzz blizzz dismissed their stale review June 15, 2023 10:38

see comment

@icewind1991
Copy link
Member

updated the sinces, included #38843

reasoning behind the backport is making nextcloud/logreader#873 available for debugging earlier as it could be very usefull during support

@blizzz blizzz mentioned this pull request Jul 10, 2023
@blizzz
Copy link
Member

blizzz commented Jul 11, 2023

needs another reviewer

@blizzz
Copy link
Member

blizzz commented Jul 13, 2023

moving to 26.0.5

@come-nc
Copy link
Contributor

come-nc commented Jul 17, 2023

Wait, why is this updating 3rdparty?

blizzz
blizzz previously requested changes Jul 17, 2023
Copy link
Member

@blizzz blizzz left a comment

Choose a reason for hiding this comment

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

doubts about 3rdparty bump

@blizzz
Copy link
Member

blizzz commented Jul 17, 2023

@ice

Wait, why is this updating 3rdparty?

@icewind1991 accidental?

@blizzz blizzz mentioned this pull request Aug 2, 2023
@blizzz
Copy link
Member

blizzz commented Aug 3, 2023

moving to 26.0.6

@blizzz blizzz mentioned this pull request Sep 5, 2023
@blizzz
Copy link
Member

blizzz commented Sep 7, 2023

moving to 26.0.7

Signed-off-by: Robin Appelman <robin@icewind.nl>
@icewind1991
Copy link
Member

fixed the accidental 3rdparty change


// inject the event dispatcher into the logger
// this is done here because there is a cyclic dependency between the event dispatcher and logger
if ($this->logger instanceof Log or $this->logger instanceof Log\PsrLoggerAdapter) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if ($this->logger instanceof Log or $this->logger instanceof Log\PsrLoggerAdapter) {
if ($this->logger instanceof Log || $this->logger instanceof Log\PsrLoggerAdapter) {

This is a backport so it should be left as-is, but we usually use ||/&& and not or/and

@icewind1991
Copy link
Member

@blizzz can you re-review?

This was referenced Sep 20, 2023
private $message;

/**
* @since 28.0.0
Copy link
Member

Choose a reason for hiding this comment

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

all @since annotations should reflect the target version of the backport

@blizzz blizzz mentioned this pull request Oct 9, 2023
Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
@blizzz blizzz enabled auto-merge October 11, 2023 08:48
@blizzz blizzz merged commit 2fc6a67 into stable26 Oct 11, 2023
37 checks passed
@blizzz blizzz deleted the backport/38490/stable26 branch October 11, 2023 10:10
* @return string
* @since 26.0.8
*/
public function getMessage(): string {
Copy link
Contributor

Choose a reason for hiding this comment

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

#38843 was not backported, wrong type.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews enhancement pending documentation This pull request needs an associated documentation update
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants