-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Update to Gradle 4.10.3/Android Gradle plugin 3.2.1 #9044
Conversation
@@ -1395,7 +1395,7 @@ | |||
<!-- Error Messages --> | |||
<!-- The following messages can\'t be factorized due to i18n --> | |||
<string name="error_refresh_posts">Posts couldn\'t be refreshed at this time</string> | |||
<string name="error_refresh_pages">Pages couldn\'t be refreshed at this time</string> | |||
<string name="error_refresh_pages" tools:ignore="UnusedResources">Pages couldn\'t be refreshed at this time</string> |
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.
If this is unused can we just get rid of the string res?
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.
It is not unused, but lint is falsely failing with an unused resources error. I don't know of another way to fix the error.
WordPress-Android/WordPress/src/main/java/org/wordpress/android/viewmodel/pages/PagesViewModel.kt
Line 184 in ea8da23
showSnackbar(SnackbarMessageHolder(string.error_refresh_pages)) |
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.
Oh I see - that's pretty odd. At first I thought the direct import of R.string
in that file was tripping up the parser, but lint still fails for that string even if I change it. Fair enough 👍
@@ -1,28 +1,6 @@ | |||
<?xml version="1.0" encoding="UTF-8"?> | |||
<issues format="4" by="lint 3.1.3"> | |||
|
|||
<issue |
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.
@jtreanor Could you explain the reasons for dropping some these entries from lint-baseline.xml
? On the face of it it's not clear why they belong in this PR, but I'm not too familiar with their background.
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.
Lint gave a bunch of warnings about issues in lint-baseline.xml
(such as the Gradle upgrade warnings) were no longer matching lint results. Some of these warnings may not have been new but I removed the ones that I saw.
Would you prefer if I didn't remove the rules that are not directly related to this PR?
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.
@jtreanor looks like they were present in develop
as well - I don't see any harm in dropping them, so let's just leave those changes in 👍 Just wanted to be clear on any background there might have been.
@@ -83,10 +83,6 @@ | |||
<ignore regexp="java-common-1.13.jar" /> | |||
</issue> | |||
|
|||
<issue id="IconLauncherFormat"> | |||
<ignore path="src/main/res/mipmap-anydpi-v26/app_icon.xml" /> | |||
</issue> |
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.
Its worth mentioning that I removed this because I got this error after upgrading:
Errors found:
/Users/james/src/tmp/WordPress-Android/WordPress: Error: Unknown issue id "IconLauncherFormat", found in /Users/james/src/tmp/WordPress-Android/WordPress/lint.xml [LintError]
So it is not necessary to ignore this issue anymore.
Looks good @jtreanor ! Everything's building and the changes make sense. I also tested Google login and building from a new clone of the repo to confirm everything's still working as expected after the changes to the - I'll leave it to you to merge it after the code freeze. |
Thanks for the thorough review @aforcier! |
Merging now since the code freeze is done. |
a15de7604a Merge pull request #25 from wordpress-mobile/deprecate-buggy-date-time-utils-methods 5026dc0904 Mark DateTimeUtils.localDateToUTC and DateTimeUtils.nowUTC as deprecated 58cd5bf239 Mark DateTimeUtils.localDateToUTC and DateTimeUtils.nowUTC as deprecated 4360e5d41f Merge pull request #23 from wordpress-mobile/update-bintray-config 48277981d8 Bump version to 1.23 81f224bd56 Fix Javadoc lint build error when releasing to Bintray 240f2492fa Update Bintray plugin to latest b6abdcae2a Merge pull request #22 from wordpress-mobile/merge-wpa 8260cdce57 Update style and lint configs 7c08103ff2 Update Gradle wrapper version 9666183e69 Merge commit 'a4a756b60bcbf144ed8527f693c4e974fa6e365d' into subtree-updates-v3 cdeb3eb7f1 Merge pull request #9215 from wordpress-mobile/feature/update-support-lib-28 e8dbd8f404 Revert update of targetSdkVersion f7014365ea Update supportLib version to 28.0.0 b3ad4c795e Fix low hanging deprecation warnings b028488305 Remove all instances of LOCATION bd194edb49 Merge pull request #9114 from wordpress-mobile/issue/8177-applog-testing e081d898e7 Merge pull request #9044 from wordpress-mobile/gradle-4-10 2ba6f52024 Move fake AppLog(the one for testing) from utils to the main project 3f74b509d0 Create copy of AppLog for unit testing purposes 78d9bfbf13 fixed merge conflict b3b6665bfd fixed merge conflict c2fa689153 Update to Gradle 4.10.3/Android Gradle plugin 3.2.1 62b03547dc Merge remote-tracking branch 'origin/develop' into feature/master-site-creation f55c4c33b1 Removed duplicated dots from image URL 818c1691c7 Rename NewSiteCreationService props ab051f9083 removed unused imports 0ea8d2aab7 moved unused getDate() method from MediaUtils to getFormattedDateForLastModified() in PostUtils 6e4cecb37b Make the progress of NewSiteCreationService indeterminate 4434057372 Merge branch 'develop' into feature/master-site-creation e7e89ad67e Fix SiteCreation segment icon color tint 2259033733 Merge branch 'develop' into feature/deep-link-main-activity 0671724495 Merge remote-tracking branch 'origin/develop' into feature/deep-linking-improvements 8f5238e5ef Reorder repositories in build.gradle to fix Gradle 4f18db4eb3 Reorder repositories in build.gradle to fix Gradle e695cb4a6b Refactor isDeepLinking to be outside MyProfileActivity d85b6dbb16 Remove unused WPImageGetter 44d63ede88 Add a new type of log tag for pages 5b4a4f09ae Move the logging wrapper class to FluxCUtils for convenience c7c1ff782a Add Crashlytics logging to make sure we're keeping track of problems uploading media d12f32a4a7 Remove coroutines and replace them with standard event driven architecture 5580689d26 Remove unused code - ImageUtils.getThumbnail git-subtree-dir: libs/utils git-subtree-split: a15de7604a34a9e989884fb84b8c9adfaeeb97b0
This updates Gradle and the Android Gradle plugin to much more recent versions. These are not quite the latest (newest plugin is 3.3.0) but its a big improvement. We can't yet update to 3.3.0 of the Gradle plugin due to an incompatibility with the Fabric Gradle plugin (see woocommerce/woocommerce-android#694). 3.3.0 is required to update to Gradle 5.
There have been many improvements since Gradle 4.4 including better performance and improved caching. This should also help build reliability (related to #9075).
Gradle release notes are here.
Android Gradle plugin release notes are here.
I will wait until after the 11.7 code freeze to merge this to reduce any risk this brings.
To test:
Update release notes:
RELEASE-NOTES.txt
.