-
Notifications
You must be signed in to change notification settings - Fork 14
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
[Build] Update Kotlin to 1.9.22 #146
Conversation
This update isn't that necessary, but should be considered anyway as it will do the following: - Align this library, to that Kotlin version, which is already used by most of this library's clients and libraries. - Unblock the usage of the dependency analysis Gradle plugin for this project. - Prepares this project for the Kotlin 2+ major update.
This change is necessary because otherwise the build fails with the below exception: ------------------------------------------------------------------------ > Task :WordPressUtils:compileDebugKotlin FAILED FAILURE: Build failed with an exception. * What went wrong: Execution failed for task ':WordPressUtils:compileDebugKotlin'. > Inconsistent JVM-target compatibility detected for tasks 'compileDebugJavaWithJavac' (1.8) and 'compileDebugKotlin' (17). Consider using JVM Toolchain: https://kotl.in/gradle/jvm/toolchain Learn more about JVM-target validation: https://kotl.in/gradle/jvm/ target-validation
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.
Do you think it's worth to add dependency analysis to this project? It has a very little number of dependencies and is not actively developed.
This PR LGTM 👍
Just like we discussed the day before, I was indeed thinking about it @wzieba , but decided to go for it. I chose in the end to go for it because this library is used as composite build for at least JP/WPAndroid, used as a dependency on all, FluxC, JP/WPAndroid and WCAndroid, plus get some contribution here-and-there from time to time, and in general it has been discussed from time to time about properly supporting this library and not letting it be on it's own... 🤷 In the end, I think I decided to at least add
I think for this initial phase of introducing the new
And with that, I think we can stop, gather some initial feedback on the overall end-to-end process and keep adding more clients/libraries when and if needed, on demand. Wdyt? 🤔 |
I don't think this should matter too much, even one extra dependency or dependency misconfiguration can cause some trouble. Obviously, more of those could increase the risk of getting into trouble, but that doesn't mean having one of those there is risk, or at least maintenance free. My goal with this plugin is to help us remove all those unused dependencies and then restrict each project to not allow any more unused dependencies from thereafter.
Btw, I think all those 6 libraries are (kind of) actively developed (from time to time), it is just that they are not being continuously developed. |
I don't see dependency-analysis-gradle-plugin printing anything troubling that could cause problems for projects that use the library, but if you think it's worth to keep such metrics for this and other libraries, I'm ok with it 👍 |
Out of curiosity @wzieba , out of these 6 below libraries, where would you use the
For example, for me, at this point, it is all about the maintainability of these libraries and me hoping that this plugin will give us advises that in time (when needed) will help us move forward, clean-up or fine-tune any dependencies. |
Frankly, I'd skip all the library projects (except FluxC due to its size and frequency of development). I think this plugin is very useful in case of large set of dependencies - removing unused dependencies and applying more fitting configurations can help in developer's experience. I'm concerned that with the library projects that are developed rarely and are small, the difference we'll gain from improving dependency configuration will be minimal and metrics we'll get from them will be mostly flat line. |
Got it, thanks, I think that I now understand more where you're coming from (I agree). 💯 To me, additional to your expectation(s):
|
Required By: #148
This PR updates Kotlin to 1.9.22.
Description
This update isn't that necessary, but should be considered anyway as it will do the following:
Test
Utils
, or via composite builds, and see if it works as expected.