Skip to content

Enable core library desugaring#14698

Merged
renanferrari merged 7 commits intofeature/blogging-remindersfrom
feature/enable-core-library-desugaring
Jun 17, 2021
Merged

Enable core library desugaring#14698
renanferrari merged 7 commits intofeature/blogging-remindersfrom
feature/enable-core-library-desugaring

Conversation

@renanferrari
Copy link
Copy Markdown
Contributor

This PR enables core library desugaring, which makes it possible for us to use Java 8+ APIs, most notably java.time and java.util.stream.

This was mainly motivated by the fact that we are going to be making quite a few operations with dates during the Notifications project and the java.time API can make that a lot less painful.

To test:

Not much to test here. Just making sure it builds and all existing tests pass should be enough.

Regression Notes

  1. Potential unintended areas of impact
    N/A

  2. What I did to test those areas of impact (or what existing automated tests I relied on)
    N/A

  3. What automated tests I added (or what prevented me from doing so)
    N/A

PR submission checklist:

  • I have completed the Regression Notes.
  • I have considered adding accessibility improvements for my changes.
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@peril-wordpress-mobile
Copy link
Copy Markdown

peril-wordpress-mobile bot commented May 21, 2021

You can trigger optional UI/connected tests for these changes by visiting CircleCI here.

@peril-wordpress-mobile
Copy link
Copy Markdown

peril-wordpress-mobile bot commented May 21, 2021

You can test the changes on this Pull Request by downloading the APK here.

@planarvoid
Copy link
Copy Markdown
Contributor

I think the lint is failing because of the Java changes.

@oguzkocer
Copy link
Copy Markdown
Contributor

I've run this lint task locally with 6GB of memory and it passed fine within a reasonable amount of time. Unfortunately in CircleCI, we are limited to 2GB of memory and it looks like that's not enough to get lint task done after these changes.

The good news is that I plan to move the lint task to BuildKite during the upcoming HACK week which will not have this restriction and the task should pass fine.

Assuming this is an optional PR, the best thing to do might be to postpone merging this PR for a bit. Sorry about that!

ravishanker added a commit that referenced this pull request May 28, 2021
Testing for #14698 lint failure issue by enabling desugaring here temporarily
Increasing heap size to see if it fixes CI heap max out issue
@renanferrari renanferrari modified the milestones: 17.5, 17.6 May 28, 2021
@renanferrari renanferrari force-pushed the feature/enable-core-library-desugaring branch from 45ebab6 to 3bd9e55 Compare May 29, 2021 00:28
…ordpress-mobile/WordPress-Android into feature/enable-core-library-desugaring
Copy link
Copy Markdown
Contributor

@planarvoid planarvoid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The app builds without problems. I'm getting the lint errors locally when running the CI command (lintWordpressVanillaRelease)

  Errors found:
  
  /Users/vojta/Development/wordpress/WordPress-Android/WordPress/src/main/java/org/wordpress/android/ui/media/MediaBrowserActivity.java:556: Error: Overriding method should call super.onRequestPermissionsResult [MissingSuperCall]
      public void onRequestPermissionsResult(int requestCode, @NonNull String[] permissions, @NonNull int[] results) {
                  ~~~~~~~~~~~~~~~~~~~~~~~~~~
  /Users/vojta/Development/wordpress/WordPress-Android/WordPress/src/main/java/org/wordpress/android/ui/media/MediaSettingsActivity.java:841: Error: Overriding method should call super.onRequestPermissionsResult [MissingSuperCall]
      public void onRequestPermissionsResult(int requestCode, @NonNull String[] permissions,
                  ~~~~~~~~~~~~~~~~~~~~~~~~~~
  /Users/vojta/Development/wordpress/WordPress-Android/WordPress/src/main/java/org/wordpress/android/ui/ShareIntentReceiverActivity.java:152: Error: Overriding method should call super.onRequestPermissionsResult [MissingSuperCall]
      public void onRequestPermissionsResult(int requestCode,
                  ~~~~~~~~~~~~~~~~~~~~~~~~~~

@AliSoftware
Copy link
Copy Markdown
Contributor

Moving this PR as well as #14719 to next milestone since 17.6 goes into code freeze today.

@AliSoftware AliSoftware modified the milestones: 17.6, 17.7 Jun 14, 2021
@renanferrari renanferrari changed the base branch from develop to feature/blogging-reminders June 17, 2021 20:57
@renanferrari renanferrari merged commit bbf0063 into feature/blogging-reminders Jun 17, 2021
@renanferrari renanferrari deleted the feature/enable-core-library-desugaring branch June 17, 2021 21:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants