-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Remove ExPlat from the project. Replace it with experimentation library from Tracks.
#22354
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
…n` library from Tracks.
Generated by 🚫 Danger |
Project dependencies changeslist+ New Dependencies
com.automattic.tracks:experimentation:6.0.6tree+\--- com.automattic.tracks:experimentation:6.0.6
+ +--- com.squareup.okhttp3:okhttp:4.12.0 -> 5.3.1 (*)
+ +--- com.squareup.moshi:moshi:1.15.1 (*)
+ +--- org.jetbrains.kotlinx:kotlinx-coroutines-core:1.6.4 -> 1.10.2 (*)
+ \--- org.jetbrains.kotlin:kotlin-stdlib:2.0.21 -> 2.2.21 (*) |
|
| App Name | WordPress | |
| Flavor | Jalapeno | |
| Build Type | Debug | |
| Version | pr22354-cd710de | |
| Commit | cd710de | |
| Direct Download | wordpress-prototype-build-pr22354-cd710de.apk |
|
| App Name | Jetpack | |
| Flavor | Jalapeno | |
| Build Type | Debug | |
| Version | pr22354-cd710de | |
| Commit | cd710de | |
| Direct Download | jetpack-prototype-build-pr22354-cd710de.apk |
Otherwise unit tests fail because Mockito has a problem with the synthethic Kotlin method (doesn't see it)
experimentation library from Tracks.experimentation library from Tracks.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## trunk #22354 +/- ##
==========================================
- Coverage 39.12% 39.07% -0.06%
==========================================
Files 2205 2199 -6
Lines 106157 106030 -127
Branches 15049 15035 -14
==========================================
- Hits 41536 41431 -105
+ Misses 61129 61111 -18
+ Partials 3492 3488 -4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| appLogWrapper: AppLogWrapper, | ||
| @Named(APPLICATION_SCOPE) appScope: CoroutineScope, | ||
| ) = VariationsRepository.create( | ||
| platform = "wpandroid", |
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.
From
WordPress-Android/libs/fluxc/src/main/java/org/wordpress/android/fluxc/store/ExperimentStore.kt
Line 89 in 69ed046
| WORDPRESS_ANDROID("wpandroid"), |
| // Possible way to initialize variations repository. Not tested, verify when working on experiments. | ||
| // variationsRepository.initialize("${tracker.anonID}") |
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.
I don't implement it because the project doesn't run any experiments, and what we consider "experiment id" can vary from use case. This is just a hint where we should likely initialize VariationsRepository.
| if (!firstActivityResumed) { | ||
| // Since we're force refreshing on app startup, we don't need to try refreshing again when starting | ||
| // our first Activity. | ||
| exPlat.refreshIfNeeded() | ||
| } |
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.
Intended removal. The experimentation library will handle per-session cache. There's no need to refresh the experiments on app resume.
jkmassel
left a comment
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.
LGTM, but if we're not doing any experiments right now would it make more sense to just remove without adding a replacement until we need it?
There are no currently running experiments, so this integration is redundant. #22354 (review)
Makes sense 👍 . And in the future, this PR could help as a reference point on how to integrate the library. I removed the |
|





Closes: AINFRA-661: Remove FluxC dependency on Track's
experimentationDescription
Update: this PR removes
ExPlat/experimentationfrom the project, as there are no running experiments.This PR is a twin of woocommerce/woocommerce-android#14128.
The idea here is to replace internal experimentation logic with
experimentationlibrary.The historical context is following:
ExPlatwas shared via FluxC between WordPress and WooCommerceexperimentationfrom WooCommerce's FluxC and replaced it thereTesting instructions
Actually - not needed. The
experimentationlibrary was tested during development. As there are no running experiments in WordPress/Jetpack, we can't meaningfully test this PR.