-
-
Notifications
You must be signed in to change notification settings - Fork 356
feat: Remove SentryOptions.enabled #736
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
Other SentrySDKs don't have this options. We want to align with them and remove it. Furthermore SentryClient now doesn't send any events or sessions if the DSN is nil.
Codecov Report
@@ Coverage Diff @@
## master #736 +/- ##
==========================================
+ Coverage 91.92% 92.06% +0.13%
==========================================
Files 71 71
Lines 3071 3061 -10
==========================================
- Hits 2823 2818 -5
+ Misses 248 243 -5
Continue to review full report at Codecov.
|
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.
Nice! deleted code replaced with testing the correct behavior!
This is really a bummer. We relied on this to easily enable/disable sending to Sentry based on user privacy settings. It doesn't appear that there is any other mechanism we can use at runtime to enable and disable sending to Sentry. |
@philipphofmann It should be noted that #700 cannot (in its current form) be used for toggling the state of Sentry based on the user's privacy preferences / consent, since it's read-only. |
Honestly the feature we’re looking for is exactly what this feature did. We need something we can toggle at runtime. If I were to propose a new feature, it would be just reverting this pull request. |
I understand there's a rational for homogenizing the different platform SDKs, but in doing so has Sentry considered the impact that the solution of removing |
📜 Description
Other SentrySDKs don't have this option. We want to align with them and remove it.
Furthermore, SentryClient now doesn't send any events or sessions if the DSN is nil.
💡 Motivation and Context
Other SentrySDKs don't have this option. We want to align with them and remove it.
💚 How did you test it?
Travis.
📝 Checklist
🔮 Next steps