-
-
Notifications
You must be signed in to change notification settings - Fork 461
Hubs/Scopes Merge 21 - Allow controlling which scope configureScope uses
#3345
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
Conversation
…pes-merge-2-add-scopes
…ainScopes to rootScopes
|
| @Override | ||
| public void configureScope(@NotNull ScopeCallback callback) { | ||
| scopes.configureScope(callback); | ||
| public void configureScope(@Nullable ScopeType scopeType, @NotNull ScopeCallback callback) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
m: I guess we should do the same for withScope
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, will add in a follow up PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, this has the same problem as having an overload for forkedScope where withScope(ISOLATION) ... forks both isolation and current scope which might be confusing.
Maybe having another method called withIsolationScope, as RFC suggests, is better but very similar.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding withIsolationScope in a follow up PR
#skip-changelog
📜 Description
A default
ScopeTypecan be set inSentryOptionswhich defaults toISOLATIONfor JVM (backend, etc.) andCURRENTfor Android. This default scope is then used forconfigureScopewhen no explicitScopeTypeis passed.💡 Motivation and Context
💚 How did you test it?
📝 Checklist
sendDefaultPIIis enabled.🔮 Next steps