Skip to content
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

chore(android): Bump version for Next storage #1028

Merged
merged 9 commits into from
Nov 28, 2023
Merged

Conversation

krizzu
Copy link
Member

@krizzu krizzu commented Nov 18, 2023

Summary

  • Bumped default Kotlin version to 1.9.20
  • Coroutines bumped to 1.7.3
  • Dropped kapt in favor of ksp, which version can be set via gradle property

@krizzu krizzu requested a review from tido64 as a code owner November 18, 2023 16:56
@krizzu krizzu mentioned this pull request Nov 18, 2023
5 tasks
@tido64 tido64 linked an issue Nov 20, 2023 that may be closed by this pull request
5 tasks
@@ -47,14 +47,19 @@ buildscript {
? rootProject.ext['kotlinVersion']
: rootProject.hasProperty('AsyncStorage_kotlinVersion')
? rootProject.properties['AsyncStorage_kotlinVersion']
: '1.8.10'
: '1.9.20'
Copy link
Member

Choose a reason for hiding this comment

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

Will this cause issues when react-native is still on 1.8? And what about older versions?

In react-native-test-app, we keep a list of compatible versions: https://github.com/microsoft/react-native-test-app/blob/e43b6a622bffaa76ce28e1ed1c4f036db6611342/android/dependencies.gradle#L6

Should we be doing the same here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Kotlin is being kept forward binary compatible, so It should be fine with older versions. Gradle will select latest kotlin version and evict the rest.

I do like your suggestion tho, I could add a map of supported KSP versions, so we can decide on it based on Kt version used

Choose a reason for hiding this comment

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

The addition of the supported KSP versions map will be particularly valuable for legacy projects, providing a clear understanding of compatibility.

@krizzu krizzu force-pushed the chore/android-version-bump branch from cd6d034 to 5a4c5ea Compare November 21, 2023 10:58
@krizzu krizzu requested a review from tido64 November 21, 2023 10:59
@krizzu
Copy link
Member Author

krizzu commented Nov 21, 2023

@tido64 Moved the config to separate file, for easier management. I left an option to still be able to override KSP version, in case Kotlin v2 and supporting KSP releases, before we add support for it.

@krizzu
Copy link
Member Author

krizzu commented Nov 21, 2023

@tido64 CI is failing due to

Plugin [id: 'com.google.devtools.ksp', version: 'null'] was not found in any of the following sources

I had same issue locally, where dependencies.gradle file could not be read from TestApp.

@tido64
Copy link
Member

tido64 commented Nov 21, 2023

@tido64 CI is failing due to

Plugin [id: 'com.google.devtools.ksp', version: 'null'] was not found in any of the following sources

I had same issue locally, where dependencies.gradle file could not be read from TestApp.

Seems like we're missing the latest version: microsoft/react-native-test-app#1701

I've also updated the logic to not return null.

Update: Can you try bumping to 2.5.33?

@krizzu krizzu force-pushed the chore/android-version-bump branch from 5a4c5ea to 9d87a32 Compare November 22, 2023 12:09
@krizzu
Copy link
Member Author

krizzu commented Nov 22, 2023

Bumped, let's give it a go 🤞

@tido64
Copy link
Member

tido64 commented Nov 22, 2023

Looks like we need to bump Robolectric:

com.reactnativecommunity.asyncstorage.next.AsyncStorageAccessTest > initializationError FAILED
    java.lang.IllegalArgumentException: failed to configure com.reactnativecommunity.asyncstorage.next.AsyncStorageAccessTest.performsBasicGetSetRemoveOperations: Package targetSdkVersion=33 > maxSdkVersion=31
        at org.robolectric.RobolectricTestRunner.getChildren(RobolectricTestRunner.java:260)

Latest version should support up to 34.

@krizzu
Copy link
Member Author

krizzu commented Nov 22, 2023

Yeah, will try to do it later day or tomorrow 🙏
Done

packages/default-storage/android/build.gradle Outdated Show resolved Hide resolved
packages/default-storage/android/build.gradle Outdated Show resolved Hide resolved
packages/default-storage/android/config.gradle Outdated Show resolved Hide resolved
Comment on lines -11 to +13
"android/build.gradle",
"android/src",
"android/testresults.gradle",
"android/",
"!android/.gradle",
"!android/build",
Copy link
Member Author

Choose a reason for hiding this comment

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

for some weird reason, the src dir was not added, so had to add all and exclude build files

Copy link
Member

Choose a reason for hiding this comment

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

This might be related to the rewrite of the glob implementation in npm 9. We've hit issues with it as well: npm/npm-packlist#152

Copy link
Member Author

Choose a reason for hiding this comment

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

not cool!

@krizzu krizzu force-pushed the chore/android-version-bump branch from 9d92442 to ab7fe04 Compare November 28, 2023 10:09
@krizzu krizzu requested a review from tido64 November 28, 2023 10:09
Copy link
Member

@tido64 tido64 left a comment

Choose a reason for hiding this comment

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

🚀

@krizzu
Copy link
Member Author

krizzu commented Nov 28, 2023

@tido64 added a log to it. I'm going to release this as minor version update, due to libraries bump

@krizzu krizzu merged commit 14489a7 into main Nov 28, 2023
9 checks passed
@krizzu krizzu deleted the chore/android-version-bump branch November 28, 2023 16:04
@AsyncStorageBot
Copy link
Collaborator

🎉 This PR is included in version 1.20.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Kotlin Compatibility
4 participants