-
Notifications
You must be signed in to change notification settings - Fork 205
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
Enable automatic session tracking by default #314
Changes from all commits
461eebb
307270c
59a8164
1b14ae9
711ab74
6886111
84efa1e
e276037
8e38483
f89c322
145c45a
4dcf7ad
3334063
66ce062
397ac7f
b03b1d6
ceb8cfe
abbcc28
58baad8
6432b4c
0706d25
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -61,6 +61,11 @@ class SessionTracker implements Application.ActivityLifecycleCallbacks { | |
* @param user the session user (if any) | ||
*/ | ||
void startNewSession(@NonNull Date date, @Nullable User user, boolean autoCaptured) { | ||
if (configuration.getSessionEndpoint() == null) { | ||
Logger.warn("The session tracking endpoint has not been set. " | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should have mentioned this earlier, sorry - Should this warning be a one-time thing? We could end up adding a lot of logger noise. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess that's ok, are there any similar cases already present in the notifier? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This one is somewhat similar. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I mean for the logging repeatedly when in debug mode? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @Cawllec it's not particularly easy to test log messages in Android unfortunately |
||
+ "Session tracking is disabled"); | ||
return; | ||
} | ||
Session session = new Session(UUID.randomUUID().toString(), date, user, autoCaptured); | ||
currentSession.set(session); | ||
trackSessionIfNeeded(session); | ||
|
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.
What's the advantage of this over the previous with the default set to
true
?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.
This method no longer requires any knowledge of whether sessions should be automatically captured or not, and relies on
Configuration
instead to specify a sensible default.