Conversation
Choose ndkVersion from ext first
Summary of ChangesHello @ntcong91, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request updates the Android build configuration to correctly retrieve the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
📝 WalkthroughWalkthroughThe pull request adds externalized NDK version configuration to the Android build file by introducing an Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request correctly configures the ndkVersion by reading it from the project's external properties, with a fallback to gradle.properties. This is a good improvement for build configuration flexibility. I have one minor suggestion regarding code formatting to improve readability.
| minSdk getExtOrIntegerDefault("minSdkVersion") | ||
| targetSdk getExtOrIntegerDefault("targetSdkVersion") | ||
| ndkVersion getExtOrDefault("ndkVersion") | ||
|
|
There was a problem hiding this comment.
This extra blank line appears to be unintentional. Removing it will improve code formatting consistency within the defaultConfig block.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@android/build.gradle`:
- Line 27: Move the ndkVersion assignment out of defaultConfig into the android
{ } block and only set it when getExtOrDefault("ndkVersion") returns a
non-null/non-empty value; specifically, locate where ndkVersion is currently
being assigned inside defaultConfig, remove that assignment, and add a guarded
assignment inside the android { } (CommonExtension) configuration that checks
the result of getExtOrDefault("ndkVersion") (or the Sound_ndkVersion project
property) and sets android.ndkVersion only when a value is present so you
preserve AGP's default behavior when no NDK version is configured.
| defaultConfig { | ||
| minSdk getExtOrIntegerDefault("minSdkVersion") | ||
| targetSdk getExtOrIntegerDefault("targetSdkVersion") | ||
| ndkVersion getExtOrDefault("ndkVersion") |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the android/build.gradle file to understand the current structure
cat -n android/build.gradle | head -50Repository: hyochan/react-native-nitro-sound
Length of output: 1997
🏁 Script executed:
# Search for all ndkVersion usages in the codebase
rg -n 'ndkVersion' --type groovy --type gradleRepository: hyochan/react-native-nitro-sound
Length of output: 290
🏁 Script executed:
# Check if there are any gradle.kts files that might also use ndkVersion
fd -e gradle.kts --exec grep -n 'ndkVersion' {}Repository: hyochan/react-native-nitro-sound
Length of output: 58
🏁 Script executed:
# Examine the getExtOrDefault function definition
cat -n android/build.gradle | grep -A 5 -B 2 'getExtOrDefault'Repository: hyochan/react-native-nitro-sound
Length of output: 1005
🏁 Script executed:
# Look for any AGP version or gradle plugin declarations
cat -n android/build.gradle | grep -E '(plugins|gradle|agp|AGP|classpath)'Repository: hyochan/react-native-nitro-sound
Length of output: 224
🏁 Script executed:
# Check the example app's build.gradle context around line 76
sed -n '70,85p' example/android/app/build.gradleRepository: hyochan/react-native-nitro-sound
Length of output: 675
🏁 Script executed:
# Get broader context to see the block structure
sed -n '1,100p' example/android/app/build.gradle | cat -nRepository: hyochan/react-native-nitro-sound
Length of output: 4763
🏁 Script executed:
# Check the root build.gradle to understand the structure better
cat -n example/android/build.gradle | head -50Repository: hyochan/react-native-nitro-sound
Length of output: 769
🏁 Script executed:
# Search for any documentation or comments about ndkVersion placement in the repo
rg -n 'ndkVersion' -B5 -A5 --type groovy --type gradleRepository: hyochan/react-native-nitro-sound
Length of output: 2047
🏁 Script executed:
# Check if there are any Gradle wrapper or AGP version declarations
find . -name "gradle-wrapper.properties" -o -name "build.gradle" | xargs grep -l "com.android.tools.build:gradle"Repository: hyochan/react-native-nitro-sound
Length of output: 106
🏁 Script executed:
# Let's verify if AGP actually throws an error for ndkVersion in defaultConfig
# by checking if there are any Kotlin source files or documentation in the repo
find . -name "*.kt" -type f | head -5Repository: hyochan/react-native-nitro-sound
Length of output: 431
🏁 Script executed:
# Check if there are any gradle plugin or AGP version hints
rg -n 'gradle.*version|agp|AGP' --type groovy | head -20Repository: hyochan/react-native-nitro-sound
Length of output: 160
🌐 Web query:
Android Gradle Plugin ndkVersion property defaultConfig vs android block placement
💡 Result:
- Official/idiomatic placement (AGP DSL):
ndkVersionis a property of the module’sandroid {}extension (i.e.,CommonExtension.ndkVersion). Set it directly insideandroid {}, not insidedefaultConfig {}. Example from the official DSL reference:android { ndkVersion '20.1.5948944' }. [1] - What happens if you don’t set it: AGP will supply a default NDK version depending on the AGP version, so upgrading AGP can implicitly change the NDK unless you lock it with
ndkVersion. [1][2] - Why
defaultConfig { ndkVersion ... }sometimes “works”: in Groovy DSL, nested blocks can sometimes resolve unknown properties via dynamic delegation, so you may see Stack Overflow answers that putndkVersionunderdefaultConfig. But it’s not the documented location, is not how the DSL is defined, and is more likely to fail in Kotlin DSL (and/or be fragile across AGP versions). Preferandroid { ndkVersion ... }. [1][2]
Recommended (Kotlin DSL):
android {
ndkVersion = "26.1.10909125"
}Recommended (Groovy DSL):
android {
ndkVersion "26.1.10909125"
}Sources: official AGP DSL (CommonExtension.ndkVersion) [1], NDK/AGP behavior notes (default NDK + android.ndkVersion) [2].
[1] https://developer.android.com/reference/tools/gradle-api/8.3/com/android/build/api/dsl/CommonExtension
[2] https://github.com/android/ndk-samples/wiki/Configure-NDK-Path
🌐 Web query:
AGP CommonExtension ndkVersion DefaultConfig API reference
💡 Result:
-
ndkVersionis a property oncom.android.build.api.dsl.CommonExtension(i.e., the top-levelandroid { ... }block for app/library/etc.), not ondefaultConfig. It “requires the specified NDK version to be used.”[major.minor.build]format, and it’s mutually exclusive withndkPath. [1] -
defaultConfig { ... }is configured viaCommonExtension.defaultConfig(...)and usesDefaultConfig/BaseFlavorAPIs. NDK-related settings insidedefaultConfigare done via thendk { ... }block (typeNdk) for things like ABI filters (per-variant/per-flavor config), not the NDK version. [1][2]
API reference pages (Android Developers):
CommonExtension (includes ndkVersion, defaultConfig):
https://developer.android.com/reference/tools/gradle-api/8.0/com/android/build/api/dsl/CommonExtension
DefaultConfig (defaultConfig DSL type; inherits ndk { ... } from BaseFlavor/VariantDimension):
https://developer.android.com/reference/tools/gradle-api/8.0/com/android/build/api/dsl/DefaultConfig
Typical Kotlin DSL usage:
android {
ndkVersion = "26.1.10909125" // CommonExtension.ndkVersion
defaultConfig {
ndk { abiFilters += setOf("arm64-v8a", "x86_64") } // DefaultConfig/BaseFlavor.ndk { ... }
}
}Sources: [1] [2]
Move ndkVersion to the android {} block and guard against null values
ndkVersion is a property of CommonExtension (the android {} block), not DefaultConfig. The Android Gradle Plugin documentation and API reference explicitly define it at the top-level android {} block. Your example app correctly places it at line 76 in example/android/app/build.gradle. Placing it inside defaultConfig {} violates the DSL contract and will cause build failures.
Additionally, getExtOrDefault("ndkVersion") can return null when neither rootProject.ext nor the Sound_ndkVersion project property is set. AGP provides a default NDK version when none is specified, so the safest approach is to skip the assignment entirely when no value is configured—which preserves the pre-PR behavior.
🐛 Proposed fix
android {
namespace "com.margelo.nitro.sound"
compileSdk getExtOrIntegerDefault("compileSdkVersion")
+ def ndkVer = getExtOrDefault("ndkVersion")
+ if (ndkVer) {
+ ndkVersion ndkVer
+ }
+
defaultConfig {
minSdk getExtOrIntegerDefault("minSdkVersion")
targetSdk getExtOrIntegerDefault("targetSdkVersion")
- ndkVersion getExtOrDefault("ndkVersion")
externalNativeBuild {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ndkVersion getExtOrDefault("ndkVersion") | |
| android { | |
| namespace "com.margelo.nitro.sound" | |
| compileSdk getExtOrIntegerDefault("compileSdkVersion") | |
| def ndkVer = getExtOrDefault("ndkVersion") | |
| if (ndkVer) { | |
| ndkVersion ndkVer | |
| } | |
| defaultConfig { | |
| minSdk getExtOrIntegerDefault("minSdkVersion") | |
| targetSdk getExtOrIntegerDefault("targetSdkVersion") | |
| externalNativeBuild { |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@android/build.gradle` at line 27, Move the ndkVersion assignment out of
defaultConfig into the android { } block and only set it when
getExtOrDefault("ndkVersion") returns a non-null/non-empty value; specifically,
locate where ndkVersion is currently being assigned inside defaultConfig, remove
that assignment, and add a guarded assignment inside the android { }
(CommonExtension) configuration that checks the result of
getExtOrDefault("ndkVersion") (or the Sound_ndkVersion project property) and
sets android.ndkVersion only when a value is present so you preserve AGP's
default behavior when no NDK version is configured.
1 participant
Choose ndkVersion from ext first
Summary by CodeRabbit