Skip to content

Update Build.gradle #33507

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.

Sign up for GitHub

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

Closed
Morritz wants to merge 1 commit into from
Closed

Update Build.gradle #33507

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions ReactAndroid/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,7 @@ task androidSourcesJar(type: Jar) {

android {
compileSdkVersion 31
ndkVersion rootProject.ext.ndkVersion
Copy link
Contributor

@leotm leotm Apr 6, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I was setting this too earlier in Android Studio manually when integrating other libraries, since it wasn't being picked up from rootProject

So i've had a yarn patch stash sitting awhile locally, but avoided committing it in the end (could jus be me)

I don't see the harm including it, but assumed it wasn't for a reason - can't think of side-effects it would cause atm

Copy link
Contributor Author

@Morritz Morritz Apr 6, 2022

Choose a reason for hiding this comment

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

I think there needs to be such change made in order to make developer's life easier. Especially in early phases of development like starting with the template which already should have as I remember correctly some ndk version put there, but if developer wants to upgrade it to newer version manually then I see no problem there. Maybe it would be better for the ReactAndroid to update this ndk as internal dependency to support M1 platform, but still someone might want to use some version that he has already installed and if there is no problem with that as you mentioned, this change would be beneficial, because it would reduce digging through the deeps of the code to set it somewhere else.

Copy link
Contributor Author

@Morritz Morritz Apr 6, 2022

Choose a reason for hiding this comment

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

Mainly it helps with ndk-build, which was also addressed in other dev commit that i have seen in repo and that change was to replace ndk-build with cmake build.

Copy link
Contributor

@leotm leotm Apr 11, 2022

Choose a reason for hiding this comment

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

Gotcha and thanks the explanation earlier ^ appreciate it

Just back from a mini-break and spotted (thx too @cortinico)

Which looks like it'll resolve our NDK issue too, I'll check later today


// Used to override the NDK path & version on internal CI
if (System.getenv("ANDROID_NDK") != null && System.getenv("LOCAL_ANDROID_NDK_VERSION") != null) {
Expand Down