-
Notifications
You must be signed in to change notification settings - Fork 130
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
[WIP] Use vendored fluxc
and login
#13304
base: login_with_history
Are you sure you want to change the base?
Conversation
Generated by 🚫 Danger |
Project dependencies changesThe following changes in project dependencies were detected (configuration list
tree-+--- org.wordpress:fluxc:trunk-657a72d968f89dc2edd6a6e6e57f579643d1e213
-| +--- org.jetbrains.kotlin:kotlin-parcelize-runtime:1.9.25 -> 2.0.21
-| | +--- org.jetbrains.kotlin:kotlin-stdlib:2.0.21 (*)
-| | \--- org.jetbrains.kotlin:kotlin-android-extensions-runtime:2.0.21
-| | \--- org.jetbrains.kotlin:kotlin-stdlib:2.0.21 (*)
-| +--- androidx.exifinterface:exifinterface:1.0.0 -> 1.3.6
-| | \--- androidx.annotation:annotation:1.2.0 -> 1.8.1 (*)
-| +--- androidx.security:security-crypto:1.0.0 -> 1.1.0-alpha03
-| | +--- androidx.annotation:annotation:1.1.0 -> 1.8.1 (*)
-| | +--- com.google.crypto.tink:tink-android:1.5.0
-| | \--- androidx.collection:collection:1.1.0 -> 1.4.0 (*)
-| +--- com.squareup.okhttp3:okhttp-urlconnection:4.9.0
-| | +--- com.squareup.okhttp3:okhttp:4.9.0 -> 4.12.0 (*)
-| | \--- org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.4.10 -> 1.9.10 (*)
-| +--- com.google.code.gson:gson:2.8.5 -> 2.10.1
-| +--- org.apache.commons:commons-text:1.10.0 (*)
-| +--- androidx.room:room-runtime:2.6.1 (*)
-| +--- androidx.room:room-ktx:2.6.1
-| | +--- androidx.room:room-common:2.6.1 (*)
-| | +--- androidx.room:room-runtime:2.6.1 (*)
-| | +--- org.jetbrains.kotlin:kotlin-stdlib:1.8.22 -> 2.0.21 (*)
-| | +--- org.jetbrains.kotlinx:kotlinx-coroutines-android:1.7.1 -> 1.8.1 (*)
-| | +--- androidx.room:room-common:2.6.1 (c)
-| | \--- androidx.room:room-runtime:2.6.1 (c)
-| +--- com.google.dagger:dagger:2.51.1
-| | \--- javax.inject:javax.inject:1
-| +--- org.jetbrains.kotlinx:kotlinx-coroutines-core:1.8.1 (*)
-| +--- org.jetbrains.kotlinx:kotlinx-coroutines-android:1.8.1 (*)
-| +--- org.wordpress:wellsql:2.0.0
-| | +--- androidx.annotation:annotation:1.2.0 -> 1.8.1 (*)
-| | \--- org.wordpress.wellsql:wellsql-annotations:2.0.0
-| +--- org.wordpress.fluxc:fluxc-annotations:trunk-657a72d968f89dc2edd6a6e6e57f579643d1e213
-| +--- org.greenrobot:eventbus:3.3.1 (*)
-| +--- org.greenrobot:eventbus-java:3.3.1
-| +--- com.squareup.okhttp3:okhttp:4.9.0 -> 4.12.0 (*)
-| +--- com.android.volley:volley:1.1.1 -> 1.2.0
-| +--- androidx.paging:paging-runtime:2.1.2
-| | +--- androidx.paging:paging-common:2.1.2
-| | | +--- androidx.annotation:annotation:1.0.0 -> 1.8.1 (*)
-| | | \--- androidx.arch.core:core-common:2.0.0 -> 2.2.0 (*)
-| | +--- androidx.arch.core:core-runtime:2.0.0 -> 2.2.0 (*)
-| | +--- androidx.lifecycle:lifecycle-runtime:2.0.0 -> 2.8.7 (*)
-| | +--- androidx.lifecycle:lifecycle-livedata:2.0.0 -> 2.8.7 (*)
-| | \--- androidx.recyclerview:recyclerview:1.0.0 -> 1.3.2 (*)
-| +--- com.goterl:lazysodium-android:5.0.2
-| +--- net.java.dev.jna:jna:5.5.0
-| \--- org.jetbrains.kotlin:kotlin-stdlib:1.9.25 -> 2.0.21 (*)
-+--- org.wordpress.fluxc.plugins:woocommerce:trunk-657a72d968f89dc2edd6a6e6e57f579643d1e213
-| +--- org.wordpress:fluxc:trunk-657a72d968f89dc2edd6a6e6e57f579643d1e213 (*)
-| +--- com.google.code.gson:gson:2.8.5 -> 2.10.1
-| +--- com.google.dagger:dagger:2.51.1 (*)
-| +--- org.jetbrains.kotlinx:kotlinx-coroutines-core:1.8.1 (*)
-| +--- org.jetbrains.kotlinx:kotlinx-coroutines-android:1.8.1 (*)
-| +--- androidx.room:room-runtime:2.6.1 (*)
-| +--- org.wordpress:wellsql:2.0.0 (*)
-| +--- org.wordpress.fluxc:fluxc-annotations:trunk-657a72d968f89dc2edd6a6e6e57f579643d1e213
-| +--- androidx.room:room-ktx:2.6.1 (*)
-| \--- org.jetbrains.kotlin:kotlin-stdlib:1.9.25 -> 2.0.21 (*)
-\--- org.wordpress:login:trunk-706a58b33ff8f1a2858ae64e51204426ec12fe7b
- +--- com.gravatar:gravatar:0.2.0
- | +--- com.squareup.okhttp3:okhttp:4.12.0 (*)
- | +--- com.squareup.retrofit2:retrofit:2.9.0
- | | \--- com.squareup.okhttp3:okhttp:3.14.9 -> 4.12.0 (*)
- | +--- com.squareup.retrofit2:converter-gson:2.9.0
- | | +--- com.squareup.retrofit2:retrofit:2.9.0 (*)
- | | \--- com.google.code.gson:gson:2.8.5 -> 2.10.1
- | +--- org.jetbrains.kotlinx:kotlinx-coroutines-android:1.7.3 -> 1.8.1 (*)
- | \--- org.jetbrains.kotlin:kotlin-stdlib:1.9.22 -> 2.0.21 (*)
- +--- androidx.appcompat:appcompat:1.6.1 (*)
- +--- androidx.constraintlayout:constraintlayout:2.0.4 -> 2.1.4 (*)
- +--- com.google.android.material:material:1.2.1 -> 1.12.0 (*)
- +--- androidx.core:core:1.12.0 -> 1.13.1 (*)
- +--- com.github.bumptech.glide:glide:4.12.0 -> 4.16.0
- | +--- com.github.bumptech.glide:gifdecoder:4.16.0
- | | \--- androidx.annotation:annotation:1.5.0 -> 1.8.1 (*)
- | +--- com.github.bumptech.glide:disklrucache:4.16.0
- | +--- com.github.bumptech.glide:annotations:4.16.0
- | +--- androidx.fragment:fragment:1.3.6 -> 1.8.5 (*)
- | +--- androidx.vectordrawable:vectordrawable-animated:1.1.0 (*)
- | +--- androidx.exifinterface:exifinterface:1.3.6 (*)
- | \--- androidx.tracing:tracing:1.0.0 (*)
- +--- androidx.credentials:credentials:1.2.0
- | +--- androidx.annotation:annotation:1.5.0 -> 1.8.1 (*)
- | +--- org.jetbrains.kotlin:kotlin-stdlib:1.8.22 -> 2.0.21 (*)
- | +--- org.jetbrains.kotlinx:kotlinx-coroutines-core:1.7.1 -> 1.8.1 (*)
- | \--- androidx.credentials:credentials-play-services-auth:1.2.0 (c)
- +--- androidx.credentials:credentials-play-services-auth:1.2.0
- | +--- androidx.credentials:credentials:1.2.0 (*)
- | +--- com.google.android.gms:play-services-auth:20.7.0 (*)
- | +--- com.google.android.gms:play-services-fido:20.1.0 (*)
- | +--- com.google.android.libraries.identity.googleid:googleid:1.1.0
- | | +--- org.jetbrains.kotlin:kotlin-stdlib:1.8.0 -> 2.0.21 (*)
- | | \--- org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.8.0 -> 1.9.10 (*)
- | +--- org.jetbrains.kotlin:kotlin-stdlib:1.8.22 -> 2.0.21 (*)
- | \--- androidx.credentials:credentials:1.2.0 (c)
- +--- com.google.dagger:dagger:2.47 -> 2.51.1 (*)
- +--- com.google.dagger:dagger-android-support:2.47 -> 2.50
- | +--- com.google.dagger:dagger:2.50 -> 2.51.1 (*)
- | +--- com.google.dagger:dagger-android:2.50
- | | +--- com.google.dagger:dagger:2.50 -> 2.51.1 (*)
- | | +--- com.google.dagger:dagger-lint-aar:2.50
- | | +--- androidx.annotation:annotation:1.2.0 -> 1.8.1 (*)
- | | \--- javax.inject:javax.inject:1
- | +--- com.google.dagger:dagger-lint-aar:2.50
- | +--- androidx.activity:activity:1.5.1 -> 1.8.1 (*)
- | +--- androidx.annotation:annotation:1.2.0 -> 1.8.1 (*)
- | +--- androidx.appcompat:appcompat:1.3.1 -> 1.6.1 (*)
- | +--- androidx.fragment:fragment:1.5.1 -> 1.8.5 (*)
- | +--- androidx.lifecycle:lifecycle-common:2.5.1 -> 2.8.7 (*)
- | +--- androidx.lifecycle:lifecycle-viewmodel:2.5.1 -> 2.8.7 (*)
- | +--- androidx.lifecycle:lifecycle-viewmodel-savedstate:2.5.1 -> 2.8.7 (*)
- | \--- javax.inject:javax.inject:1
- +--- com.google.android.gms:play-services-auth:18.1.0 -> 20.7.0 (*)
- \--- org.jetbrains.kotlin:kotlin-stdlib:1.9.25 -> 2.0.21 (*)
++--- project :libs:fluxc
+| +--- org.wordpress:wellsql:2.0.0
+| | +--- androidx.annotation:annotation:1.2.0 -> 1.8.1 (*)
+| | \--- org.wordpress.wellsql:wellsql-annotations:2.0.0
+| +--- project :libs:fluxc-annotations
+| +--- org.greenrobot:eventbus:3.3.1 (*)
+| +--- org.greenrobot:eventbus-java:3.3.1
+| +--- com.squareup.okhttp3:okhttp:4.9.0 -> 4.12.0 (*)
+| +--- com.android.volley:volley:1.1.1 -> 1.2.0
+| +--- androidx.paging:paging-runtime:2.1.2
+| | +--- androidx.paging:paging-common:2.1.2
+| | | +--- androidx.annotation:annotation:1.0.0 -> 1.8.1 (*)
+| | | \--- androidx.arch.core:core-common:2.0.0 -> 2.2.0 (*)
+| | +--- androidx.arch.core:core-runtime:2.0.0 -> 2.2.0 (*)
+| | +--- androidx.lifecycle:lifecycle-runtime:2.0.0 -> 2.8.7 (*)
+| | +--- androidx.lifecycle:lifecycle-livedata:2.0.0 -> 2.8.7 (*)
+| | \--- androidx.recyclerview:recyclerview:1.0.0 -> 1.3.2 (*)
+| +--- com.goterl:lazysodium-android:5.0.2
+| +--- net.java.dev.jna:jna:5.5.0
+| +--- org.jetbrains.kotlin:kotlin-stdlib:2.0.21 (*)
+| +--- androidx.exifinterface:exifinterface:1.0.0 -> 1.3.6
+| | \--- androidx.annotation:annotation:1.2.0 -> 1.8.1 (*)
+| +--- androidx.security:security-crypto:1.0.0 -> 1.1.0-alpha03
+| | +--- androidx.annotation:annotation:1.1.0 -> 1.8.1 (*)
+| | +--- com.google.crypto.tink:tink-android:1.5.0
+| | \--- androidx.collection:collection:1.1.0 -> 1.4.0 (*)
+| +--- org.wordpress:utils:3.15.0 (*)
+| +--- com.squareup.okhttp3:okhttp-urlconnection:4.9.0
+| | +--- com.squareup.okhttp3:okhttp:4.9.0 -> 4.12.0 (*)
+| | \--- org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.4.10 -> 1.9.10 (*)
+| +--- com.google.code.gson:gson:2.10.1
+| +--- org.apache.commons:commons-text:1.10.0 (*)
+| +--- androidx.room:room-runtime:2.6.1 (*)
+| +--- androidx.room:room-ktx:2.6.1
+| | +--- androidx.room:room-common:2.6.1 (*)
+| | +--- androidx.room:room-runtime:2.6.1 (*)
+| | +--- org.jetbrains.kotlin:kotlin-stdlib:1.8.22 -> 2.0.21 (*)
+| | +--- org.jetbrains.kotlinx:kotlinx-coroutines-android:1.7.1 -> 1.8.1 (*)
+| | +--- androidx.room:room-common:2.6.1 (c)
+| | \--- androidx.room:room-runtime:2.6.1 (c)
+| +--- com.google.dagger:dagger:2.50 -> 2.51.1
+| | \--- javax.inject:javax.inject:1
+| +--- org.jetbrains.kotlinx:kotlinx-coroutines-core:1.8.1 (*)
+| +--- org.jetbrains.kotlinx:kotlinx-coroutines-android:1.8.1 (*)
+| \--- org.jetbrains.kotlin:kotlin-parcelize-runtime:2.0.21
+| +--- org.jetbrains.kotlin:kotlin-stdlib:2.0.21 (*)
+| \--- org.jetbrains.kotlin:kotlin-android-extensions-runtime:2.0.21
+| \--- org.jetbrains.kotlin:kotlin-stdlib:2.0.21 (*)
++--- project :libs:fluxc-plugin
+| +--- org.wordpress:wellsql:2.0.0 (*)
+| +--- project :libs:fluxc-annotations
+| +--- androidx.room:room-ktx:2.6.1 (*)
+| +--- org.jetbrains.kotlin:kotlin-stdlib:2.0.21 (*)
+| +--- project :libs:fluxc (*)
+| +--- org.wordpress:utils:3.15.0 (*)
+| +--- com.google.code.gson:gson:2.10.1
+| +--- com.google.dagger:dagger:2.50 -> 2.51.1 (*)
+| +--- org.jetbrains.kotlinx:kotlinx-coroutines-core:1.8.1 (*)
+| +--- org.jetbrains.kotlinx:kotlinx-coroutines-android:1.8.1 (*)
+| \--- androidx.room:room-runtime:2.6.1 (*)
+\--- project :libs:login
+ +--- com.google.android.gms:play-services-auth:20.2.0 -> 20.7.0 (*)
+ +--- org.jetbrains.kotlin:kotlin-stdlib:2.0.21 (*)
+ +--- org.wordpress:utils:3.15.0 (*)
+ +--- com.gravatar:gravatar:0.2.0
+ | +--- com.squareup.okhttp3:okhttp:4.12.0 (*)
+ | +--- com.squareup.retrofit2:retrofit:2.9.0
+ | | \--- com.squareup.okhttp3:okhttp:3.14.9 -> 4.12.0 (*)
+ | +--- com.squareup.retrofit2:converter-gson:2.9.0
+ | | +--- com.squareup.retrofit2:retrofit:2.9.0 (*)
+ | | \--- com.google.code.gson:gson:2.8.5 -> 2.10.1
+ | +--- org.jetbrains.kotlinx:kotlinx-coroutines-android:1.7.3 -> 1.8.1 (*)
+ | \--- org.jetbrains.kotlin:kotlin-stdlib:1.9.22 -> 2.0.21 (*)
+ +--- androidx.appcompat:appcompat:1.4.2 -> 1.6.1 (*)
+ +--- androidx.constraintlayout:constraintlayout:2.1.4 (*)
+ +--- com.google.android.material:material:1.12.0 (*)
+ +--- androidx.core:core-ktx:1.13.1 (*)
+ +--- project :libs:fluxc (*)
+ +--- com.github.bumptech.glide:glide:4.16.0
+ | +--- com.github.bumptech.glide:gifdecoder:4.16.0
+ | | \--- androidx.annotation:annotation:1.5.0 -> 1.8.1 (*)
+ | +--- com.github.bumptech.glide:disklrucache:4.16.0
+ | +--- com.github.bumptech.glide:annotations:4.16.0
+ | +--- androidx.fragment:fragment:1.3.6 -> 1.8.5 (*)
+ | +--- androidx.vectordrawable:vectordrawable-animated:1.1.0 (*)
+ | +--- androidx.exifinterface:exifinterface:1.3.6 (*)
+ | \--- androidx.tracing:tracing:1.0.0 (*)
+ +--- androidx.credentials:credentials:1.2.0
+ | +--- androidx.annotation:annotation:1.5.0 -> 1.8.1 (*)
+ | +--- org.jetbrains.kotlin:kotlin-stdlib:1.8.22 -> 2.0.21 (*)
+ | +--- org.jetbrains.kotlinx:kotlinx-coroutines-core:1.7.1 -> 1.8.1 (*)
+ | \--- androidx.credentials:credentials-play-services-auth:1.2.0 (c)
+ +--- androidx.credentials:credentials-play-services-auth:1.2.0
+ | +--- androidx.credentials:credentials:1.2.0 (*)
+ | +--- com.google.android.gms:play-services-auth:20.7.0 (*)
+ | +--- com.google.android.gms:play-services-fido:20.1.0 (*)
+ | +--- com.google.android.libraries.identity.googleid:googleid:1.1.0
+ | | +--- org.jetbrains.kotlin:kotlin-stdlib:1.8.0 -> 2.0.21 (*)
+ | | \--- org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.8.0 -> 1.9.10 (*)
+ | +--- org.jetbrains.kotlin:kotlin-stdlib:1.8.22 -> 2.0.21 (*)
+ | \--- androidx.credentials:credentials:1.2.0 (c)
+ +--- com.google.dagger:dagger:2.50 -> 2.51.1 (*)
+ \--- com.google.dagger:dagger-android-support:2.50
+ +--- com.google.dagger:dagger:2.50 -> 2.51.1 (*)
+ +--- com.google.dagger:dagger-android:2.50
+ | +--- com.google.dagger:dagger:2.50 -> 2.51.1 (*)
+ | +--- com.google.dagger:dagger-lint-aar:2.50
+ | +--- androidx.annotation:annotation:1.2.0 -> 1.8.1 (*)
+ | \--- javax.inject:javax.inject:1
+ +--- com.google.dagger:dagger-lint-aar:2.50
+ +--- androidx.activity:activity:1.5.1 -> 1.8.1 (*)
+ +--- androidx.annotation:annotation:1.2.0 -> 1.8.1 (*)
+ +--- androidx.appcompat:appcompat:1.3.1 -> 1.6.1 (*)
+ +--- androidx.fragment:fragment:1.5.1 -> 1.8.5 (*)
+ +--- androidx.lifecycle:lifecycle-common:2.5.1 -> 2.8.7 (*)
+ +--- androidx.lifecycle:lifecycle-viewmodel:2.5.1 -> 2.8.7 (*)
+ +--- androidx.lifecycle:lifecycle-viewmodel-savedstate:2.5.1 -> 2.8.7 (*)
+ \--- javax.inject:javax.inject:1 Build environment changesThe following changes in the build classpath were detected: list
tree+\--- org.jetbrains.kotlin.kapt:org.jetbrains.kotlin.kapt.gradle.plugin:2.0.21
+ \--- org.jetbrains.kotlin:kotlin-gradle-plugin:2.0.21 (*) |
📲 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.
|
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.
Android Lint found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
df275bc
to
a17b932
Compare
54707c5
to
11253d1
Compare
bd9aece
to
bb71676
Compare
11253d1
to
0cb655c
Compare
bb71676
to
565d952
Compare
0cb655c
to
2f75caf
Compare
565d952
to
4048993
Compare
7b5ebd2
to
5d08958
Compare
4048993
to
db27b1d
Compare
This PR introduces necessary changes to make the `assembleWasabiDebug` pass when using vendored `fluxc` and `login` modules. Among others, this PR: - replaces coordinates of `fluxc` and `login` to point to local modules - updates dependencies and adds missing ones for vendored modules - unifies JVM version - adds necessary dependencies - fixes minor code issues, forced by Kotlin update on vendored codebase
Which contains all unit tests from original `example` module. Also, this commit adds a number of changes to make unit tests run. `ArrayUtilsTest` is removed as `ArrayUtils` was an example-app only class.
- `mockito` upgrade is necessary to make the build work with newer JDKs ``` Java 21 (65) is not supported by the current version of Byte Buddy which officially supports Java 20 (64) - update Byte Buddy or set net.bytebuddy.experimental as a VM property ``` - since 5.0, `mockito-core` includes the `mockito-inline` module
To address `NoWhenBranchMatchedException`: these failing tests were crashing because `response` in `ProductRestClient#fetchProducts` was checked in `when` branch, while the value was null (not mocked). This change was requested by Kotlin update.
Citing the documentation: ``` The List type in Java is mapped to the MutableList type in Kotlin. Because the List.removeFirst() and List.removeLast() APIs have been introduced in Android 15 (API level 35), the Kotlin compiler resolves function calls, for example list.removeFirst(), statically to the new List APIs instead of to the extension functions in kotlin-stdlib. If an app is re-compiled with compileSdk set to 35 and minSdk set to 34 or lower, and then the app is run on Android 14 and lower, a runtime error is thrown: ``` https://developer.android.com/about/versions/15/behavior-changes-15
They were not enabled/configured back in FluxC, so enabling them here causes hundreds of violations. Adjust `MagicNumber` rule to also fit newly added codebase
5d08958
to
eaeaacf
Compare
…-tests` and `login` modules This option was not enabled originally.
cc2d648
to
ffd798b
Compare
@ParaskP7 this PR is ready for the first round of reviews. The chain of branches is following:
I'm working right now on running benchmarks to make sure that we don't experience any performance regression with incremental builds. After this, and syncing with Woo Android team, this work will be ready to be merged. |
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.
👋 @wzieba !
I have reviewed and (quickly) tested this PR as per the instructions, everything works as expected, another amazing job well done, kudos! 🌟 x 🌟 ^ 🌟
PS: Thanks for adding the script
you run for the extractions/filtering of fluxc
and login
.
I have left a question (❓), a few suggestions (💡) and some minor (🔍) comments for you to consider. I am going to approve this PR anyway, since none is blocking. I am NOT going to merge this PR yet to give you some time to apply any of my suggestions. However, feel free to ignore them and merge the PR yourself.
EXTRAS
- About all the comments I added in my review, 18 in total, I know, but all I did is to gather the
TODOs
for you. I am sure that you already planned for most of those changes, but I didn't want to risk missing some of those, losing par with JP/WPAndroid. - I didn't do a thorough smoke test of the project, just a quick one. I am hoping that, after you apply any of the comment included in this review, someone from the WCAndroid team will jump on it with a thorough smoke test and give us a final 🟢 light for the merge.
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.
Minor (🔍): Some things I noticed during review, some related, some unrelated to the actual changes in this PR:
- Related: The
androidx-appcompat
version is pointing to1.4.2
but thelogin
library was pointing to1.6.1
, we could consider updating it to point to a latest version instead of an older one. PS: I am sure that transitive updates already pointing to a much newer version, but... 🤷 - Unrelated: The
androidx-activity
version is unused, we could consider removing it. - Unrelated: The
android-billingclient-ktx
andbumptech-glide-ksp
libraries are unused, we could consider removing them.
wordpress-libaddressinput = '0.0.2' | ||
wordpress-lint = "2.0.0" |
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.
Minor (🔍): The wordpress-lint
version is pointing to 2.0.0
but the login
library was pointing to 2.1.0
, we could consider updating it to point to a latest version instead of an older one.
sourceCompatibility = JavaVersion.VERSION_1_8 | ||
targetCompatibility = JavaVersion.VERSION_1_8 | ||
sourceCompatibility libs.versions.java.get() | ||
targetCompatibility libs.versions.java.get() | ||
|
||
withSourcesJar() | ||
withJavadocJar() |
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.
Minor (🔍): Consider removing this configuration above.
PS: Same applies for the fluxc-processor
module.
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.
Question (❓): I want to question this testInstrumentationRunner
here and this one MigrationTests
related test.
- I am not sure it is being kept up-to-date, see the latest test being the
testMigration36to37
withWC_DATABASE_VERSION
pointing to38
already. 🤔 - Maybe we should remove this UI/connected test and any
androidTest
configuration. And if not, maybe we should make this test part of the CI pipeline? 🤔
remoteComments.removeFirst() | ||
remoteComments.removeAt(0) | ||
remoteComments.removeAt(0) | ||
remoteComments.removeAt(0) |
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.
Praise (❤️): Nicely done!
FYI: I've searched through the codebase and I found this last reference of RollingLogEntries.add(...) -> removeFirst(). Maybe something to look into? 🤔
@@ -1,123 +1,90 @@ | |||
import org.jetbrains.kotlin.gradle.tasks.KotlinCompile | |||
|
|||
plugins { |
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.
Suggestion (💡): How about converting this to alias
to keep the consistency between modules:
alias(libs.plugins.android.library)
alias(libs.plugins.kotlin.android)
alias(libs.plugins.kotlin.kapt)
@@ -1,123 +1,90 @@ | |||
import org.jetbrains.kotlin.gradle.tasks.KotlinCompile | |||
|
|||
plugins { | |||
id "com.android.library" | |||
id "org.jetbrains.kotlin.android" | |||
id "org.jetbrains.kotlin.kapt" |
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.
Suggestion (💡): Replacing kapt
with ksp
for this module is quite simple, maybe we should consider it.
PS: Then, we could also remove the allWarningsAsErrors = false
related configuration from this module.
Minor (🔍): Btw @wzieba I forgot to mention in my review above that we would also want to also remove the local-builds.gradle configuration for both the |
Description
This PR introduces necessary changes to make the
assembleWasabiDebug
pass when using vendoredfluxc
andlogin
modules.Among others, this PR:
fluxc
andlogin
to point to local modulesThe current state of PR is based on code from following commits
Extraction from FluxC and Login
To perform extractions/filtering, I've used the following script.
Caution
DO NOT RUN the script, it's only for preview and understanding what actions has been performed via
git filter-repo
script
Testing information
The whole app needs to be tested: nothing in particular.
The tests that have been performed
Images/gif
RELEASE-NOTES.txt
if necessary. Use the "[Internal]" label for non-user-facing changes.Reviewer (or Author, in the case of optional code reviews):
Please make sure these conditions are met before approving the PR, or request changes if the PR needs improvement: