-
Notifications
You must be signed in to change notification settings - Fork 780
Allow the override of compileSdkVersion, buildToolsVersion, minSdkVersion and targetSdkVersion #527
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
Conversation
|
@jamespearson Since you're replacing several other commits, it would also be useful to change It'll fix an issue that's arisen between RN 0.57 and RN 0.58: facebook/react-native#22033 (comment) |
…sion and targetSdkVersion
|
Hi @jamespearson, thanks so much for this! It seems like a great way to resolve this ongoing issue. Could you add a section to the readme explaining with that configuration snippet and explaining how to use it? |
| } | ||
|
|
||
| dependencies { | ||
| compileOnly 'com.facebook.react:react-native:+' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First of all, thanks for the contribution. I'm waiting for this PR to be merged. And It would be better if you replace "compileOnly" with "implementation".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @abdurrahmanekr, could you include a link that explains why that’s better?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @paulmelnikow, I was actually going to say what @gtebbutt said.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be merged this needs some documentation in the readme. Would you be up for adding that? You could make a new branch off master with these commits and then add the documentation.
Would be great to get this merged!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I would like to contribute but I don't think the use of this should be written on README because it is now available on most plugins. Because the user does not need to make any settings. The "buildscript.ext" variable in the <project dir> /android/build.gradle file is actually "rootProject.ext". This link can be an example. You want a sentence like I wrote?
Allow versions to be set via config, rather than hard coded. This is becoming the standard for React Native modules.
Config example, in build.gradle:
This would also remove the need for the following to be merged: #520 #502 #487 #459