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

fix gradle setup #227

Merged
merged 1 commit into from
Sep 16, 2019
Merged

fix gradle setup #227

merged 1 commit into from
Sep 16, 2019

Conversation

dulmandakh
Copy link
Contributor

Importing Android Gradle Plugin in android/build.gradle is causing issues if version mismatch with root project. This PR tried to fix gradle setup, and moving most of the setup from android to project root.

@mikehardy
Copy link

I'm following this from some discussion with @SaeedZhiany on a different issue, and I don't see how this gets rid of the gradle version in the module android/build.gradle, it seems like it just moves it to .kts files. How does this actually help solve the related issue?

I would love the ability to have no gradle version specified in my module's android/build.gradle so the app drove the overall version

@dulmandakh
Copy link
Contributor Author

I'll try to elaborate on the change, sorry for late reply.

android folder is used in RN app as a module, so having Android Gradle Plugin in it causes version conflict related issues. So it removed Android Gradle Plugin from android/build.gradle file.

but to build and test the module code, we need a project to wrap it, so I created a build.gradle.kts in its parent to make it project, which includes android module. So in Android Studio, you no longer open android folder as project, but it's parent.

@dulmandakh
Copy link
Contributor Author

also moved wrapper to parent directory or project.

@mikehardy
Copy link

Ah interesting - so think of this as active listening where I check my understanding - for this to really solve the problem it would need to be similar to the ext{} block of versions, where modules would all need to adopt this style where as a module maintainer you sort of used a new / available "namespace" / gradle project space for module work, and doing so it means there is no longer gradle version stuff in a place where the react-native project build sees it.

If I got that correct I could see this working, but I could also see the transition taking some time to propagate out to the module ecosystem...

@dulmandakh
Copy link
Contributor Author

@mikehardy yep, it'll take some time to propagate

@dulmandakh
Copy link
Contributor Author

@vonovak please review 👍

@mikehardy
Copy link

@vonovak As a fellow library maintainer I have a hard time keeping up with all the current styles an fashion in the react-native world, but I have to say after reviewing all the related discussions on this one with @dulmandakh and @SaeedZhiany, this actually looks like the right way to go (and the way I will go with my modules), even though I was a little skeptical at first. At the very least, it moves you up from gradle 1.x :-)

// This source code is licensed under the MIT license found in the
// LICENSE file in the root directory of this source tree.

buildscript {

Choose a reason for hiding this comment

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

This is the cool part - because the build script is in kotlin (which Android Studio recognizes just fine) but is in the module root, you can build your module just fine for testing - with specific versions - but when people import it into their work project, your project definition doesn't really exist. They import it using their project's gradle versions.

@SaeedZhiany SaeedZhiany mentioned this pull request Sep 16, 2019
@vonovak vonovak merged commit 1b8d84d into oblador:master Sep 16, 2019
@vonovak
Copy link
Collaborator

vonovak commented Sep 16, 2019

Wish I could have more time for OSS now to dive more into this; thanks for the PR! This is not a breaking change, correct?

@dulmandakh
Copy link
Contributor Author

yep, not a breaking change.

@SaeedZhiany
Copy link

Is it possible to revert these changes and use the @friederbluemle's idea in this comment?

@friederbluemle
Copy link
Contributor

Yes, it should be possible: #263

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.

5 participants