-
-
Notifications
You must be signed in to change notification settings - Fork 210
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
Add auto session tracking #1068
Conversation
I thought it would be best if we only started sending session data on a major release (as we did for Android, JavaScript, iOS) and until then had it as a opt-in feature. On a server application we'd definitely want that turned off so we need to add to the ASP.NET and ASP.NET Core packages as We can always flip the flag later on, lets start with it as |
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.
Looks good other than we should default to false
and again on aspnet and aspnetcore default to false again because we'll flip this to true
on the core next major.
On the comment we should say: Default is false
but will be changed to true
on the next major release.
Updated. Not sure why the tests are failing (seems unrelated but who knows). I don't really have time to debug it today, but I will look into it next week (unless you want to poke at it yourself). |
Codecov Report
@@ Coverage Diff @@
## main #1068 +/- ##
==========================================
- Coverage 80.69% 80.64% -0.05%
==========================================
Files 194 195 +1
Lines 6284 6294 +10
Branches 1397 1399 +2
==========================================
+ Hits 5071 5076 +5
- Misses 757 760 +3
- Partials 456 458 +2
Continue to review full report at Codecov.
|
CI passed. thanks |
Related to #1054
Implemented through
ISdkIntegration
.Tests omitted.
Note the issue says:
Should we do that? I added it as
true
by default because I thought that was the consensus.