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.

Sign up for GitHub

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

Jump to bottom

Hook for BaseScope.Apply #596

Merged
merged 2 commits into from
Nov 20, 2020
Merged

Hook for BaseScope.Apply #596

merged 2 commits into from
Nov 20, 2020

Conversation

Tyrrrz
Copy link
Contributor

@Tyrrrz Tyrrrz commented Nov 19, 2020 •
edited
Loading

Related to #594

@bruno-garcia I've looked at how existing Func<IEnumerable<ISentryEventProcessor>>[] and etc. work, but they don't make sense to me. Can you guide me in regards to what I should do to make it usable from ASP.NET Core?

@Tyrrrz Tyrrrz added Feature New feature or request Sentry labels Nov 19, 2020
@Tyrrrz Tyrrrz requested a review from bruno-garcia as a code owner November 19, 2020 15:27
@Tyrrrz
Copy link
Contributor Author

Tyrrrz commented Nov 19, 2020

Open question: what should happen in the event of scope.IsLocked? Should the apply be called on the current scope? Should it only happen if the custom hook was provided?

@bruno-garcia
Copy link
Member

Open question: what should happen in the event of scope.IsLocked? Should the apply be called on the current scope? Should it only happen if the custom hook was provided?

yeah we just set the data to the current scope.

bruno-garcia
bruno-garcia reviewed Nov 20, 2020
View reviewed changes
Copy link
Member

@bruno-garcia bruno-garcia left a comment

Choose a reason for hiding this comment

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

Looking good!

Given that the high level API is X<TScope>(T arg I wonder if we could do the same here and dump the object arg

src/Sentry.Protocol/BaseScopeExtensions.cs Show resolved Hide resolved
@bruno-garcia bruno-garcia merged commit f2abba1 into stable-backport Nov 20, 2020
@bruno-garcia bruno-garcia deleted the scope-apply-hook branch November 20, 2020 18:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bruno-garcia bruno-garcia bruno-garcia left review comments

Assignees
No one assigned
Labels
Feature New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Tyrrrz @bruno-garcia