Conversation
This 'compileTestKotlin' configuration block seems unnecessary. The corresponding 'compileKotlin' configuration block should be all that is needed in that case. Also, there is no 'test' source associated with this module, and as such this makes the 'compileTestKotlin' configuration block even more unnecessary.
This configuration was removed from both, the 'annotations' and 'processors' modules. This configuration was removed as a follow-up on this discussion below when its was discussed whether we should be removing the JVM source/target explicitly at that point. After a short discussion, it was decided to not do that back then, but nowadays there is no much point in keeping this configuration any longer. Discussion: wordpress-mobile/WordPress-iOS#19335
There is no other such 'compileOptions' configuration on any other 'libs' related module, only on the 'apps' related 'WordPress' module. As such, this configuration on the 'editor' related libs module seems redundant and thus removed.
This configuration already exist on the root level 'build.gradle'
configuration file, which applies it by default to all modules within
the 'tasks.withType(KotlinCompile).all { ... }' block.
2 tasks
Contributor
|
You can test the WordPress changes on this Pull Request by downloading an installable build (wordpress-installable-build-pr17195-b7704dd.apk), or scanning this QR code: |
Contributor
|
You can test the Jetpack changes on this Pull Request by downloading an installable build (jetpack-installable-build-pr17195-b7704dd.apk), or scanning this QR code: |
3 tasks
Contributor
Author
The honour is all mine @ovitrif , thanks for the review and testing, you rock! 🌟 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Parent: #17173
Closes: #17174
Associated To: #17197
This PR enables all warnings as errors for the
annotationsmodule.This
allWarningsAsErrorsconfiguration is currently applied on the module level in order to make sure that, as the overall Compiler Warnings as Errors work is progressing, no new warnings are added to this module, which is already free of warnings.When the overall Compiler Warnings as Errors work is complete on all modules, then this module level
allWarningsAsErrorsconfiguration will be replaced by a root level such configuration that will be applied by default to all modules (see here).Note that, in addition to enabling the
allWarningsAsErrorsconfiguration on theannotationsmodule, I took this opportunity to improve on other aspects ofjavaandbuildrelated configurations. As such, this PR also improves on the following in order to mainstream this (kind of related) group of configurations, for this and all other lib module:javajava 7source/targetCompatibilityconfigurationcompileTestKotlinkotlinOptionscompileOptionsreposrepositoriesPS: @ovitrif I added you as the main reviewer, that is, in addition to @wordpress-mobile/apps-infrastructure team itself, but randomly, since I want someone from the
WordPress Androidteam to primarily sign-off on that change. 🥇FYI: I am going to randomly add more of you in those PRs that will follow, just so you become more aware of this change and how close we are on enabling
allWarningsAsErrorsby default everywhere. 🎉To test:
annotationsmodule, along with theprocessorsmodule is responsible for that. For example, you could try switching thejetpack_powered_bottom_sheet_remote_fieldfeature flag, on and off, and see if that works as expected.Regression Notes
The feature flag functionality is not workings as expected.
See
To testsection above.N/A
PR submission checklist:
RELEASE-NOTES.txtif necessary.