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

Update build.gradle setup #209

Merged

Conversation

friederbluemle
Copy link
Contributor

Summary

A slightly different approach to #207

This wraps the Android Gradle plugin dependency in the buildscripts section of android/build.gradle in a conditional:

if (project == rootProject) {
    // ... (dependency here)
}

The Android Gradle plugin is only required when opening the project stand-alone, not when it is included as a dependency. By doing this, the project opens correctly in Android Studio, and it can also be consumed as a native module dependency from an application project without affecting the app project (avoiding unnecessary downloads/conflicts/etc).

Test Plan

What's required for testing (prerequisites)?

Android Studio and/or Gradle installed locally.

What are the steps to reproduce (after prerequisites)?

Open android in Android Studio:

studio android/

Or:

Generate a Gradle wrapper on the fly, and build on command line:

cd android/
gradle wrapper --gradle-version 5.4.1 --distribution-type all
./gradlew build

Successfully verified and tested on:

  • Android Device (Xiaomi Mi A2)
  • Android Emulator (Pixel 3a)
  • iOS Device (iPhone 8)
  • iOS Simulator (iPhone X)

Once approved, please use a normal GitHub merge (i.e. NO rebase/squash merge) to integrate the commit(s) from the PR head branch. The changes are broken up into meaningful, atomic commits, and my branch should already be up-to-date with the latest base branch. If it isn't, or if you want me to change anything, please let me know, and I will update the branch as soon as possible. Thank you!

Copy link
Member

@krizzu krizzu left a comment

Choose a reason for hiding this comment

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

Thanks for your input @friederbluemle. I like this approach more - opting out instead of removing.

Also, please remove getExtOrDefault and ** getExtOrIntegerDefault** functions, as they're no longer used.

Out of curiosity - why would you open this lib as a standalone project? It makes more sense to test it directly in Android env, by running app and using the module (Unless you're writing some tests or gradle scripts).

android/build.gradle Outdated Show resolved Hide resolved
android/build.gradle Show resolved Hide resolved
@friederbluemle
Copy link
Contributor Author

@krizzu - Thanks for the review. I addressed your comments, and also removed the two "getExtOrDefault" (I thought I removed them earlier, but missed it ;))
Regarding opening it stand-alone: Sometimes it can be useful to validate and compile the project in isolation, without any other projects or apps involved. It would also be possible to add unit tests that specifically target library code (and of course don't need an app project).

android/build.gradle Outdated Show resolved Hide resolved
This also updates Android Gradle plugin to 3.5.0.
@krizzu krizzu merged commit b54ba79 into react-native-async-storage:LEGACY Sep 18, 2019
@krizzu
Copy link
Member

krizzu commented Sep 18, 2019

Thanks a lot @friederbluemle @dulmandakh

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

Successfully merging this pull request may close these issues.

None yet

3 participants