Skip to content

Deprecate the logger option - #1167

Merged
ste93cry merged 2 commits into
getsentry:developfrom
ste93cry:deprecate-logger-option
Jan 12, 2021
Merged

ste93cry merged 2 commits into
getsentry:developfrom
ste93cry:deprecate-logger-option

Conversation

@ste93cry

@ste93cry ste93cry commented Jan 9, 2021

Copy link
Copy Markdown
Contributor

As mentioned in #1165, while fixing a bug involving the logger option I found out that at some point in time both the JS and Python SDK stopped supporting it (or they never did, to be honest I don't have the full historic context to say it). Anyway, since the code of those libraries is mainly our reference and I don't think that having this option adds much value as there is usually one logger per application, I decided to deprecate the option and remove it in the next major version. The field with the same name in the event object is not affected.

@Jean85 Jean85 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM 👍 I would require a 👍 from someone like @HazAT too to be sure that removing this option doesn't have some strange side-effect or limitation server-side.

Comment thread src/Options.php Outdated
@Jean85
Jean85 requested a review from HazAT January 11, 2021 07:50

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

Kill Deprecate it with 🔥

@stayallive stayallive left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🔥 🚢

@ste93cry
ste93cry merged commit 8b89414 into getsentry:develop Jan 12, 2021
@ste93cry
ste93cry deleted the deprecate-logger-option branch January 12, 2021 10:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants