-
Notifications
You must be signed in to change notification settings - Fork 135
Introduce pos module
#14972
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
Introduce pos module
#14972
Conversation
Showcase of mooving a dependency-less class to the new module
So the test is close to implementation
So the test is close to implementation
Project manifest changes for WooCommerce-WearThe following changes in the --- ./build/reports/diff_manifest/WooCommerce-Wear/vanillaRelease/base_manifest.txt 2025-11-17 16:26:16.928754284 +0000
+++ ./build/reports/diff_manifest/WooCommerce-Wear/vanillaRelease/head_manifest.txt 2025-11-17 16:26:29.528782826 +0000
@@ -298,12 +298,12 @@
</receiver>
<activity
+ android:name="androidx.compose.ui.tooling.PreviewActivity"
+ android:exported="true" />
+ <activity
android:name="com.google.android.gms.common.api.GoogleApiActivity"
android:exported="false"
- android:theme="@android:style/Theme.Translucent.NoTitleBar" />
- <activity
- android:name="androidx.compose.ui.tooling.PreviewActivity"
- android:exported="true" /> <!-- 'android:authorities' must be unique in the device, across all apps -->
+ android:theme="@android:style/Theme.Translucent.NoTitleBar" /> <!-- 'android:authorities' must be unique in the device, across all apps -->
<provider
android:name="io.sentry.android.core.SentryInitProvider"
android:authorities="com.woocommerce.android.SentryInitProvider"Go to https://buildkite.com/automattic/woocommerce-android/builds/33833/canvas?sid=019a929f-1a19-4ab3-9137-ee483caa7274, click on the |
Project dependencies changeslist+ New Dependencies
org.glassfish:javax.annotation:10.0-b28tree-+--- project :libs:commons
-| +--- org.jetbrains.kotlin:kotlin-stdlib:2.2.21 (*)
-| \--- org.jetbrains.kotlin:kotlin-parcelize-runtime:2.2.21 (*)
++--- project :libs:pos
+| +--- org.jetbrains.kotlin:kotlin-stdlib:2.2.21 (*)
+| +--- project :libs:commons
+| | +--- org.jetbrains.kotlin:kotlin-stdlib:2.2.21 (*)
+| | +--- androidx.annotation:annotation:1.9.1 (*)
+| | +--- org.glassfish:javax.annotation:10.0-b28
+| | +--- org.jetbrains.kotlinx:kotlinx-coroutines-core:1.10.2 (*)
+| | +--- com.automattic.tracks:crashlogging:6.0.6 (*)
+| | +--- androidx.lifecycle:lifecycle-process:2.9.4 (*)
+| | +--- com.google.dagger:hilt-android:2.57.2
+| | | +--- com.google.dagger:dagger:2.57.2 (*)
+| | | +--- com.google.dagger:dagger-lint-aar:2.57.2
+| | | +--- com.google.dagger:hilt-core:2.57.2
+| | | | +--- com.google.dagger:dagger:2.57.2 (*)
+| | | | +--- com.google.code.findbugs:jsr305:3.0.2
+| | | | +--- jakarta.inject:jakarta.inject-api:2.0.1
+| | | | \--- javax.inject:javax.inject:1
+| | | +--- com.google.code.findbugs:jsr305:3.0.2
+| | | +--- androidx.activity:activity:1.5.1 -> 1.9.1 (*)
+| | | +--- androidx.annotation:annotation:1.7.1 -> 1.9.1 (*)
+| | | +--- androidx.annotation:annotation-experimental:1.3.1 -> 1.5.0 (*)
+| | | +--- androidx.fragment:fragment:1.5.1 -> 1.8.9 (*)
+| | | +--- androidx.lifecycle:lifecycle-common:2.5.1 -> 2.9.4 (*)
+| | | +--- androidx.lifecycle:lifecycle-viewmodel:2.5.1 -> 2.9.4 (*)
+| | | +--- androidx.lifecycle:lifecycle-viewmodel-savedstate:2.5.1 -> 2.9.4 (*)
+| | | +--- androidx.savedstate:savedstate:1.2.0 -> 1.3.3 (*)
+| | | +--- javax.inject:javax.inject:1
+| | | \--- org.jetbrains.kotlin:kotlin-stdlib:2.1.21 -> 2.2.21 (*)
+| | +--- org.wordpress:utils:3.15.0 (*)
+| | \--- org.jetbrains.kotlin:kotlin-parcelize-runtime:2.2.21 (*)
+| \--- com.google.dagger:hilt-android:2.57.2 (*)
-\--- com.google.dagger:hilt-android:2.57.2
- +--- com.google.dagger:dagger:2.57.2 (*)
- +--- com.google.dagger:dagger-lint-aar:2.57.2
- +--- com.google.dagger:hilt-core:2.57.2
- | +--- com.google.dagger:dagger:2.57.2 (*)
- | +--- com.google.code.findbugs:jsr305:3.0.2
- | +--- jakarta.inject:jakarta.inject-api:2.0.1
- | \--- javax.inject:javax.inject:1
- +--- com.google.code.findbugs:jsr305:3.0.2
- +--- androidx.activity:activity:1.5.1 -> 1.9.1 (*)
- +--- androidx.annotation:annotation:1.7.1 -> 1.9.1 (*)
- +--- androidx.annotation:annotation-experimental:1.3.1 -> 1.5.0 (*)
- +--- androidx.fragment:fragment:1.5.1 -> 1.8.9 (*)
- +--- androidx.lifecycle:lifecycle-common:2.5.1 -> 2.9.4 (*)
- +--- androidx.lifecycle:lifecycle-viewmodel:2.5.1 -> 2.9.4 (*)
- +--- androidx.lifecycle:lifecycle-viewmodel-savedstate:2.5.1 -> 2.9.4 (*)
- +--- androidx.savedstate:savedstate:1.2.0 -> 1.3.3 (*)
- +--- javax.inject:javax.inject:1
- \--- org.jetbrains.kotlin:kotlin-stdlib:2.1.21 -> 2.2.21 (*)
++--- project :libs:commons (*)
+\--- com.google.dagger:hilt-android:2.57.2 (*) |
📲 You can test the changes from this Pull Request in WooCommerce-Wear Android by scanning the QR code below to install the corresponding build.
|
|
📲 You can test the changes from this Pull Request in WooCommerce Android by scanning the QR code below to install the corresponding build.
|
This fixes the ClassCastException when running WooCommerce tests after moving WooLog to the commons module. Changes: 1. Remove Hilt/KSP plugins from commons module - commons doesn't need to process Hilt annotations itself. The WooCommerce and Wear modules that depend on commons will process any Hilt annotations found in commons (like @Inject on WooFileLogger). 2. Replace EntryPoints.get() pattern with direct injection - Changed WooLog.init() to accept WooFileLogger directly instead of using Context and EntryPoints.get(). This is cleaner and removes the code smell of using EntryPoints for something that can be injected. 3. Update WooCommerce's AppConfigModule to inject WooFileLogger into provideWooLog() and pass it to WooLog.init(). This approach avoids forcing the Wear module to provide dependencies (AppCoroutineScope, CoroutineDispatchers) that it doesn't actually use, since Wear doesn't use WooLog at all. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
db5a86b to
c87d136
Compare
| private lateinit var fileLogger: WooFileLogger | ||
|
|
||
| fun init(context: Context) { | ||
| fun init(fileLogger: WooFileLogger) { |
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.
Extra caution: I'm not sure why EntryPoints.get(context, FileLoggerEntryPoint::class.java).fileLogger() was introduced instead of directly passing WooFileLogger. It was added in 41e9dc0 but I couldn't find the reason there on in PR. So I assume, this was just a cosmetic choice to use context + EntryPoints.
So here I refactored it to make it simpler. This also moves injection logic from the commons module, leaving only annotations here.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## trunk #14972 +/- ##
============================================
- Coverage 38.48% 38.47% -0.01%
+ Complexity 10240 10239 -1
============================================
Files 2155 2156 +1
Lines 122167 122258 +91
Branches 16807 16812 +5
============================================
+ Hits 47013 47042 +29
- Misses 70388 70449 +61
- Partials 4766 4767 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
samiuelson
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, thanks!
Description
This PR introduces
posmodule. The goal of introducing one is to improve developer experience of the POS by shortening feedback loops.Code that is required to be used by both
WooCommerceandposshould be placed incommonmodule. You can read more about it internally: p91TBi-dCL-p2Test Steps
Smoke test the app, review code - especially the part with
WooFileLoggerinjection I commented below.Images/gif
RELEASE-NOTES.txtif necessary. Use the "[Internal]" label for non-user-facing changes.