-
Notifications
You must be signed in to change notification settings - Fork 29
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
feat: implement the analytics SDK as a singleton #424
feat: implement the analytics SDK as a singleton #424
Conversation
android/src/main/java/com/rudderstack/android/RudderAnalytics.kt
Outdated
Show resolved
Hide resolved
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 effort, requesting some changes for alignment and for better kotlin implementation.
android/src/main/java/com/rudderstack/android/RudderAnalytics.kt
Outdated
Show resolved
Hide resolved
android/src/test/java/com/rudderstack/android/RudderAnalyticsTest.kt
Outdated
Show resolved
Hide resolved
...-android/src/main/kotlin/com/rudderstack/android/sampleapp/analytics/RudderAnalyticsUtils.kt
Outdated
Show resolved
Hide resolved
android/src/main/java/com/rudderstack/android/RudderAnalytics.kt
Outdated
Show resolved
Hide resolved
… be used in the future
So that it could have a greater visibility from other modules.
Quality Gate passedIssues Measures |
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.
Simplified singleton instance and moving some ext functions to better suited place.
Great work!! Looks good to me!
About the new feature
getInstance
method. We have made sure thatgetInstance
doesn't returnnull
.About the change
createInstance
API togetInstance
and made it return a singleton instance. This logic has been moved into the newRudderAnalytics
class.getInstance
method which can returnnull
.AnalyticsUtil
into theutilities
package.Type of change
Checklist: