Update target and compile sdk version to v29 by malinajirka · Pull Request #12947 · wordpress-mobile/WordPress-Android

malinajirka · 2020-09-14T12:18:01Z

This PR updates compile and target sdk version to v29. I checked official android docs, but I haven't found any breaking change which would affect our app - privacy changes and behavior changes.

Updates some parts of our code since the nullable/nonnullable annotations were added to some of the java classes in the SDK.
Suppresses "Field requires API level 29: in MediaStore.MediaColumns" warning. The fields we use were just moved to a super class in the SDK - the constants remain the same.
Updates gutenberg-mobile reference - corresponding PRs - gutenberg-mobile, gutenberg

Not ready for merge - related prs in gutenberg repos need to be merged first and the ref needs to be updated

I think the lint is still failing since the changes in gutenberg repo are in a fork - when I targeted the submodule to the fork locally the lint passed without issues.

To test:
Test the app - anything you can think of. Ideally spent at least 10 minutes testing various flows.

PR submission checklist:

I have considered adding unit tests where possible.
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 · 2020-09-14T12:18:10Z

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

peril-wordpress-mobile · 2020-09-14T15:41:37Z

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

peril-wordpress-mobile · 2020-09-18T11:30:50Z

	Messages
📖	This PR contains changes in the subtree `libs/utils/`. It is your responsibility to ensure these changes are merged back into `wordpress-mobile/WordPress-Utils-Android`. Follow these handy steps! WARNING: Make sure your git version is 2.19.x or lower - there is currently a bug in later versions that will corrupt the subtree history! `cd WordPress-Android` `git checkout update-target-sdk-to-29` `git subtree push --prefix=libs/utils/ https://github.com/wordpress-mobile/WordPress-Utils-Android.git merge/WordPress-Android/12947` Browse to https://github.com/wordpress-mobile/WordPress-Utils-Android/pull/new/merge/WordPress-Android/12947 and open a new PR.

Generated by 🚫 dangerJS

mchowning · 2020-09-21T22:27:03Z

I think the lint is still failing since the changes in gutenberg repo are in a fork - when I targeted the submodule to the fork locally the lint passed without issues.

The lint error looks legitimate to me:

/home/circleci/project/libs/gutenberg-mobile/gutenberg/packages/react-native-aztec/android/src/main/java/org/wordpress/mobile/ReactNativeAztec/ReactAztecManager.java:374: Error: Field requires API level 29 (current min is 21): android.graphics.text.LineBreaker#JUSTIFICATION_MODE_INTER_WORD [InlinedApi]
view.setJustificationMode(LineBreaker.JUSTIFICATION_MODE_INTER_WORD);
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/circleci/project/libs/gutenberg-mobile/gutenberg/packages/react-native-aztec/android/src/main/java/org/wordpress/mobile/ReactNativeAztec/ReactAztecManager.java:379: Error: Field requires API level 29 (current min is 21): android.graphics.text.LineBreaker#JUSTIFICATION_MODE_NONE [InlinedApi]
view.setJustificationMode(LineBreaker.JUSTIFICATION_MODE_NONE);

I left a comment on the gutenberg PR about this.

mchowning · 2020-09-21T22:35:56Z

.../src/main/java/org/wordpress/android/ui/stats/refresh/lists/widget/utils/StatsIntentUtils.kt

                .takeUnless { it == -1 }
-                ?.let { T::class.java.enumConstants[it] } ?: default
+                ?.let {
+                    @Suppress("UNNECESSARY_SAFE_CALL")


Is this @Suppress needed?

Android studio complains when it's not there - I believe it's a false positive.

Interestingly Android Studio doesn't seem to give me any warning if I remove the @Suppress line like you did in the screenshot. 🤔

Certainly makes sense to include the @Suppress since you're seeing the warning though. 👍

mchowning

Tested a number of different flows and everything looks good to me.

⚠️ There is currently a crash when loading the block editor that is unrelated to this pr: p1600864334005800-slack-C6UJ0KRKQ. I expect that will be fixed soon and we just want to retest the block editor on this PR after the gutenberg-mobile submodule ref is updated to make sure we bring in the fix and not just the breakage (basically just make sure the block editor doesn't crash on load).

malinajirka · 2020-09-24T08:03:19Z

I expect that will be fixed soon and we just want to retest the block editor on this PR after the gutenberg-mobile submodule ref is updated to make sure we bring in the fix and not just the breakage (basically just make sure the block editor doesn't crash on load).

The mentioned PR has been closed (not merged). A fix is included in wordpress-mobile/gutenberg-mobile#2662. We'll need to wait until this new PR gets merged.

malinajirka · 2020-09-24T12:58:48Z

@mchowning I've updated the gutenberg-mobile submodule and merged develop into this branch. I've smoke tested the editor + some other parts of the app and all seems to work as expected. Can I just merge the PR when CI finishes or do you want to check the changes before the merge?

Thank you so much for all your help!!!!! 🙇 🙇 🙇

Update targetSdkVersion to v29 and fix breaking changes

ce5459c

malinajirka added the Tech Debt label Sep 14, 2020

malinajirka added this to the 15.8 milestone Sep 14, 2020

malinajirka requested review from aforcier and planarvoid September 14, 2020 12:18

malinajirka force-pushed the update-target-sdk-to-29 branch from 05f0634 to ce5459c Compare September 14, 2020 12:39

Update WPStories reference

a45c16a

malinajirka added the [Status] Not Ready for Review label Sep 14, 2020

Merge branch 'develop' into update-target-sdk-to-29

757d55a

malinajirka marked this pull request as draft September 15, 2020 07:50

malinajirka added 3 commits September 18, 2020 11:58

Merge branch 'develop' into update-target-sdk-to-29

00528f8

Use SDK 29 on CI

b787488

Update compile and target sdk in dependencies

0d10e6a

Suppress false positive lint issue

2764db4

malinajirka modified the milestones: 15.8, 15.9 Sep 21, 2020

This was referenced Sep 21, 2020

Update target and compile Android sdk versions to v29 WordPress/gutenberg#25488

Merged

Update gutenberg ref wordpress-mobile/gutenberg-mobile#2646

Merged

Update gutenberg-mobile ref

836074c

malinajirka added [Status] Not Ready for Merge and removed [Status] Not Ready for Review labels Sep 21, 2020

malinajirka requested a review from hypest September 21, 2020 12:43

malinajirka marked this pull request as ready for review September 21, 2020 12:44

mchowning reviewed Sep 21, 2020

View reviewed changes

Update gutenberg-mobile ref

dce0bba

malinajirka removed the [Status] Not Ready for Merge label Sep 23, 2020

malinajirka added the [Status] Not Ready for Merge label Sep 23, 2020

malinajirka requested a review from mchowning September 23, 2020 09:48

mchowning approved these changes Sep 23, 2020

View reviewed changes

malinajirka removed the [Status] Not Ready for Merge label Sep 23, 2020

malinajirka added 2 commits September 24, 2020 14:45

Update gutenberg ref

0a6592b

Merge branch 'develop' into update-target-sdk-to-29

2fcb61a

malinajirka merged commit e43f9d7 into develop Sep 24, 2020

malinajirka deleted the update-target-sdk-to-29 branch September 24, 2020 13:09

mzorz mentioned this pull request Dec 3, 2020

Use a fallback default video resolution when it can't be obtained from mediaMetadataExrtactor Automattic/stories-android#621

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update target and compile sdk version to v29#12947

Update target and compile sdk version to v29#12947
malinajirka merged 11 commits intodevelopfrom
update-target-sdk-to-29

malinajirka commented Sep 14, 2020 •

edited

Loading

Uh oh!

peril-wordpress-mobile bot commented Sep 14, 2020 •

edited

Loading

Uh oh!

peril-wordpress-mobile bot commented Sep 14, 2020 •

edited

Loading

Uh oh!

peril-wordpress-mobile bot commented Sep 18, 2020

Uh oh!

mchowning commented Sep 21, 2020

Uh oh!

mchowning Sep 21, 2020

Uh oh!

malinajirka Sep 23, 2020

Uh oh!

mchowning Sep 23, 2020 •

edited

Loading

Uh oh!

mchowning left a comment •

edited

Loading

Uh oh!

malinajirka commented Sep 24, 2020

Uh oh!

malinajirka commented Sep 24, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

malinajirka commented Sep 14, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

peril-wordpress-mobile bot commented Sep 14, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

peril-wordpress-mobile bot commented Sep 14, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

peril-wordpress-mobile bot commented Sep 18, 2020

Uh oh!

mchowning commented Sep 21, 2020

Uh oh!

mchowning Sep 21, 2020

Choose a reason for hiding this comment

Uh oh!

malinajirka Sep 23, 2020

Choose a reason for hiding this comment

Uh oh!

mchowning Sep 23, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mchowning left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

malinajirka commented Sep 24, 2020

Uh oh!

malinajirka commented Sep 24, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

malinajirka commented Sep 14, 2020 •

edited

Loading

peril-wordpress-mobile bot commented Sep 14, 2020 •

edited

Loading

peril-wordpress-mobile bot commented Sep 14, 2020 •

edited

Loading

mchowning Sep 23, 2020 •

edited

Loading

mchowning left a comment •

edited

Loading