Update `minSdkVersion` (from `21` to `24`) #1017

ParaskP7 · 2022-11-18T13:37:00Z

This PR upgrades FluxC to minSdkVersion to 24. This has been unblocked because:

WPAndroid was already on minSdkVersion = 24, for some time now (see here).
DOAndroid has been recently upgraded to minSdkVersion = 24 as well (here and commit).

As part of this minSdkVersion = 24 upgrade the below changes were also applied in order to fix any additional warnings that this change brought-up:

Warnings Resolution List:

Test List:

Warning (⚠️): Please be very mindful of this fix test failures change and verify that this is not a breaking change of some sorts. See commit description for more info.

Test

There is nothing much to test here.
Verifying that all the CI checks are successful should be enough.
However, if you want to be thorough about reviewing this change, you could:
1. Quickly smoke test the example app and see if it is working as expected.
2. Perform a quick check in gutenberg using this PR as a double-check. Cc @fluiddot

Review

@fluiddot as per the discussion on updating and testing gutenberg as well, this might be a good opportunity for you see what's coming to Aztec. As such, it might be good if we merge this change along with merging the equivalent minSdkVersion = 24 upgrade on gutenberg, that is, when that gets ready by you. PS: there is no time pressure here.

PS: Also, and based on this comment of yours, it might be better for your to perform a quick check in gutenberg using this PR instead, and not #1016 in order to save yourself some time, as this one includes that in it as well.

Merge instructions

Wait for [Compile Warnings As Errors] Resolve Warnings & Enable All Warnings as Errors #1016 to be approved and merged to trunk.
Make sure this PR's base is automatically updated to trunk.
Update PR with latest trunk.
Wait for confirmation from @fluiddot that this PR is fully tested with gutenberg and can be merged.
Remove [PR] Not Ready For Merge] label.
Merge PR to trunk.

Make sure strings will be translated:

If there are new strings that have to be translated, I have added them to the client's strings.xml as a part of the integration PR.

This will now make sure that from now on all modules will be pointing to the same 'min/compile/targetSdkVersion' versions, making such upgrades much more straightforward as that will apply to all modules. This will alleviate the risk of forgetting to update a specific module, or set of modules due to an oversight.

A quick analysis showed that updating to 'minSdkVersion = 24' added two 'ObsoleteSdkInt' new warnings on the 'aztec' library module. All these newly created warnings will be dealt with in subsequent commits. Also, any existing such warnings will be resolved too.

This '@config(sdk = [23])' is now outdated due to the 'minSdkVersion' update to '24'. As such, this configuration is being removed from everywhere. Also, and whenever necessary the '@config(sdk = [21, 25])' configuration is being updated to '@config(sdk = [24, 25])' for the same reason.

Those test, and more so, their specific 'expected' outcome are being documented so that when they get fixed, it becomes evident what was changed and why. This is done in order to make it easier to reason about what went wrong and whether main source code needs to be updated as well, that is, since there was no other explicit change but the 'minSdkVersion' to '24' update that caused those failures.

Some of the failures were caused due to the '<em>' and '<strong>' needing to be the other way around. Other failures were caused due to the '<i>' and '<s>' needing to be the other way around. While, some more failure were caused due to the '<a href="xyz">', '<b>' and/or '<s>' needing to be the other way around. This test failures are most probably related to Robolectric, and the fact that the '@config(sdk = [23])' had to be removed, thus implicitly updated, and all that, due to the 'minSdkVersion' to '24' change. However, there is also a slight possibility that this is a breaking change due to the 'minSdkVersion' to '24' update. Thus, this change need to be verified and tested.

Warning Message: "Unnecessary; SDK_INT is always >= 24" These 'VERSION.SDK_INT >= Build.VERSION_CODES.M/N' checks are no longer necessary as the 'minSdkVersion' is now '24'. As such, the if condition has been simplified.

Warning Message: "Unnecessary; SDK_INT is always >= 24" These 'tools:targetApi="jelly_bean_mr1"' and 'tools:targetApi="lollipop"' attributes are no longer necessary as the 'minSdkVersion' is now '24'.

fluiddot

@ParaskP7 I finally managed to spare some time to check this today 🙂. I tested the Aztec version using the value 1017-29e499974e6e2c450112ce3dd74c86b033e78cd6 in Gutenberg and the build failed because the minSdkVersion in Gutenberg is 21 (reference). Therefore, we'd also have to bump the minSdkVersion there to 24 in order to include these changes.

Apart from that, I did a quick check locally by manually changing the minSdkVersion to 24 and didn't find any issue when testing. However, I'd like to devote further time to this before confirming that everything is ok. As we discussed internally, I won't have the bandwidth to take a look until Nov 25th, 2022. I'll let you know when I have the results, thanks 🙇 !

NOTE: Changes from this PR and #1016 could be merged if it's blocking other projects, Aztec is versioned in Gutenberg so this won't impact until we bump the Aztec version there.

ParaskP7 · 2022-11-21T08:28:20Z

👋 @fluiddot !

@ParaskP7 I finally managed to spare some time to check this today 🙂.

😃 You are the best, thank you for finding this spare time, now enjoy your AFK without thinking about it, we can back to that after your return! 🙇

I tested the Aztec version using the value 1017-29e499974e6e2c450112ce3dd74c86b033e78cd6 in Gutenberg and the build failed because the minSdkVersion in Gutenberg is 21 (reference). Therefore, we'd also have to bump the minSdkVersion there to 24 in order to include these changes.

Yea, that's absolutely expected. 👍

Apart from that, I did a quick check locally by manually changing the minSdkVersion to 24 and didn't find any issue when testing.

Thank for for bumping Gutenberg's minSdkVersion to 24 to test these changes, I am glad you didn't find any issues, at least at this stage. 🙇

However, I'd like to devote further time to this before confirming that everything is ok. As we discussed internally, I won't have the bandwidth to take a look until Nov 25th, 2022. I'll let you know when I have the results, thanks 🙇 !

This is totally fine, please enjoy your AFK. After you're back we can pick it up were we left it and continue from there, this is not urgent work. 💯

NOTE: Changes from this PR and #1016 could be merged if it's blocking other projects, Aztec is versioned in Gutenberg so this won't impact until we bump the Aztec version there.

Thank you for saying that, but as these changes are not urgent, nor blocking other projects, at least as far as I am aware, I suggest we do it right and once, after everything is tested, as supposed to, and only then merge these changes to trunk, for both PRs. 🙏

…id into build/update-min-sdk-version-to-24

…id into build/update-min-sdk-version-to-24 � Conflicts: � aztec/src/test/kotlin/org/wordpress/aztec/PreformatTest.kt

fluiddot

LGTM 🎊 !

I checked the changes in Gutenberg both in the demo app and using WordPress-Android and I haven't found any issues.

fluiddot · 2022-11-25T19:10:49Z

@fluiddot as per the discussion on updating and testing gutenberg as well, this might be a good opportunity for you see what's coming to Aztec.

@ParaskP7 I'd like to note that I couldn't check what's coming to Aztec in Gutenberg. In relation to this, I'm wondering if we should create a GitHub release for releasing a new Aztec version with all the changes, wdyt?

As such, it might be good if we merge this change along with merging the equivalent minSdkVersion = 24 upgrade on gutenberg, that is, when that gets ready by you. PS: there is no time pressure here.

Good point, I'll prepare a PR next week to bump minSdkVersion to version 24 in Gutenberg in order to match the same version. Note that since we're not bumping the Aztec version in Gutenberg yet, it's safe to merge this PR. In a follow-up PR, once we release a new Aztec version, I'll bump the Aztec version there.

ParaskP7 · 2022-11-28T10:14:11Z

👋 @fluiddot !

Thank you so much for the review and testing, and on both, Aztec plus Gutenberg, you are awesome! 🙇 ❤️ 🥇

@ParaskP7 I'd like to note that I couldn't check what's coming to Aztec in Gutenberg. In relation to this, I'm wondering if we should create a GitHub release for releasing a new Aztec version with all the changes, wdyt?

Yes, I think this is a very good idea Carlos, as soon as we merge this PR, I'll go ahead and create a new Aztec release. 💯

Good point, I'll prepare a PR next week to bump minSdkVersion to version 24 in Gutenberg in order to match the same version.

Perfect, thank you Carlos! 🙇

Afterwards, and when that new version of Gutenberg get released, which would contain the minSdkVersion = 24 update and the Aztec update, I also recommend that on WPAndroid we then update both, the aztecVersion alongside the gutenbergMobileVersion this time. Wdyt? 🤔

Note that since we're not bumping the Aztec version in Gutenberg yet, it's safe to merge this PR. In a follow-up PR, once we release a new Aztec version, I'll bump the Aztec version there.

Perfecto, let me then merge this PR now and create a new Aztec version for you, I'll let you know when that is done! 🙏

fluiddot · 2022-11-28T10:22:46Z

Afterwards, and when that new version of Gutenberg get released, which would contain the minSdkVersion = 24 update and the Aztec update, I also recommend that on WPAndroid we then update both, the aztecVersion alongside the gutenbergMobileVersion this time. Wdyt? 🤔

Yes, usually when we upgrade the Aztec version in Gutenberg we also do it in WordPress-Android, so, all Aztec references point to the same version 👍 .

Perfecto, let me then merge this PR now and create a new Aztec version for you, I'll let you know when that is done! 🙏

Great, thank you @ParaskP7 !

ParaskP7 · 2022-11-28T10:32:46Z

Yes, usually when we upgrade the Aztec version in Gutenberg we also do it in WordPress-Android, so, all Aztec references point to the same version 👍 .

Coolio, you are the best! 🚀 🥇 🙇

Great, thank you @ParaskP7 !

Thank YOU @fluiddot and FYI the new release tag in now ready: v1.6.3

fluiddot · 2022-12-02T17:29:23Z

@ParaskP7 I'm afraid I won't have the bandwidth to update Aztec and the minSdkVersion in Gutenberg soon, not sure if that would block other work. If so, please let me know and I'll try to prioritize it. Otherwise, I estimate I could work on this at some point in the next two weeks.

ParaskP7 · 2022-12-05T08:08:11Z

👋 @fluiddot and thanks for the heads-up! ❤️

Please don't feel stressed about this update of minSdkVersion in Gutenberg, it is not blocking anything on our side, so do prioritize this update as per your schedule and availability. Given the current ongoing WP->JP migration, I would even recommend you forget about this and come back to it next month. 💯

FYI: The only blocker I am envisioning is on your side, in terms that if Gutenberg is not updated soon, next time you would want to use a newer version of Aztec, then this would not be possible and force you to first do the minSdkVersion update. But, I know you know that anyway. Apart from that, everything on our side is under control and this pending work on Gutenberg is not blocking us, so please don't worry about that, not at all. 👍

fluiddot · 2022-12-05T18:56:48Z

Great, thank you very much @ParaskP7 for elaborating on the potential blockers regarding not updating Gutenberg 🙇. I wouldn't like to postpone it too much, but I'm glad to know that it's not blocking other work.

FYI: The only blocker I am envisioning is on your side, in terms that if Gutenberg is not updated soon, next time you would want to use a newer version of Aztec, then this would not be possible and force you to first do the minSdkVersion update. But, I know you know that anyway. Apart from that, everything on our side is under control and this pending work on Gutenberg is not blocking us, so please don't worry about that, not at all. 👍

Good point, we'll note that.

fluiddot · 2023-01-31T14:11:36Z

To close the loop: Here are the PRs in Gutenberg to update minSdkVersion and bump Aztec dependency:

Once they are approved and merged, I'll create an alpha version of Gutenberg Mobile and update the Aztec version in WordPress-Android.

ParaskP7 · 2023-01-31T15:43:52Z

Great, thanks for the update and for working on that @fluiddot ! 🚀 ❤️ 🙇

ParaskP7 added 7 commits November 17, 2022 17:04

Analysis: Resolve obsolete sdk int lint warnings

de2dba2

Warning Message: "Unnecessary; SDK_INT is always >= 24" These 'VERSION.SDK_INT >= Build.VERSION_CODES.M/N' checks are no longer necessary as the 'minSdkVersion' is now '24'. As such, the if condition has been simplified.

Analysis: Resolve obsolete sdk int lint warnings for all xml files

29e4999

Warning Message: "Unnecessary; SDK_INT is always >= 24" These 'tools:targetApi="jelly_bean_mr1"' and 'tools:targetApi="lollipop"' attributes are no longer necessary as the 'minSdkVersion' is now '24'.

ParaskP7 added enhancement Tooling labels Nov 18, 2022

ParaskP7 self-assigned this Nov 18, 2022

ParaskP7 requested review from fluiddot and a team November 18, 2022 13:57

ParaskP7 marked this pull request as ready for review November 18, 2022 13:58

fluiddot reviewed Nov 18, 2022

View reviewed changes

fluiddot mentioned this pull request Nov 18, 2022

[Compile Warnings As Errors] Resolve Warnings & Enable All Warnings as Errors #1016

Merged

1 task

ParaskP7 added the [PR] Not Ready for Merge label Nov 21, 2022

Base automatically changed from analysis/all-warnings-as-errors to trunk November 21, 2022 13:01

Merge branch 'trunk' of github.com:wordpress-mobile/AztecEditor-Andro…

3383dbb

…id into build/update-min-sdk-version-to-24

ParaskP7 mentioned this pull request Nov 21, 2022

Update kotlin and other libraries and fix the code #982

Closed

1 task

ParaskP7 added 3 commits November 23, 2022 15:20

Merge branch 'trunk' of github.com:wordpress-mobile/AztecEditor-Andro…

99270bb

…id into build/update-min-sdk-version-to-24 � Conflicts: � aztec/src/test/kotlin/org/wordpress/aztec/PreformatTest.kt

Analysis: Resolve post merge assert equals deprecated warning

6a69526

Analysis: Suppress post merge unused parameter warning

1e717df

fluiddot approved these changes Nov 25, 2022

View reviewed changes

ParaskP7 removed the [PR] Not Ready for Merge label Nov 28, 2022

ParaskP7 merged commit a28fe23 into trunk Nov 28, 2022

ParaskP7 deleted the build/update-min-sdk-version-to-24 branch November 28, 2022 10:15

ParaskP7 mentioned this pull request Nov 28, 2022

[Build] Resolve Post Merge Warnings #1018

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update `minSdkVersion` (from `21` to `24`) #1017

Update `minSdkVersion` (from `21` to `24`) #1017

ParaskP7 commented Nov 18, 2022 •

edited

Loading

fluiddot left a comment

ParaskP7 commented Nov 21, 2022

fluiddot left a comment

fluiddot commented Nov 25, 2022

ParaskP7 commented Nov 28, 2022

fluiddot commented Nov 28, 2022

ParaskP7 commented Nov 28, 2022

fluiddot commented Dec 2, 2022

ParaskP7 commented Dec 5, 2022

fluiddot commented Dec 5, 2022

fluiddot commented Jan 31, 2023

ParaskP7 commented Jan 31, 2023

Update minSdkVersion (from 21 to 24) #1017

Update minSdkVersion (from 21 to 24) #1017

Conversation

ParaskP7 commented Nov 18, 2022 • edited Loading

Test

Review

Merge instructions

fluiddot left a comment

Choose a reason for hiding this comment

ParaskP7 commented Nov 21, 2022

fluiddot left a comment

Choose a reason for hiding this comment

fluiddot commented Nov 25, 2022

ParaskP7 commented Nov 28, 2022

fluiddot commented Nov 28, 2022

ParaskP7 commented Nov 28, 2022

fluiddot commented Dec 2, 2022

ParaskP7 commented Dec 5, 2022

fluiddot commented Dec 5, 2022

fluiddot commented Jan 31, 2023

ParaskP7 commented Jan 31, 2023

Update `minSdkVersion` (from `21` to `24`) #1017

Update `minSdkVersion` (from `21` to `24`) #1017

ParaskP7 commented Nov 18, 2022 •

edited

Loading