-
Notifications
You must be signed in to change notification settings - Fork 61
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(native-app): Android build fixes #17211
Conversation
WalkthroughThe changes in this pull request involve modifications to several configuration files for an Android application. Key updates include adjustments to directory paths in the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
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.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
apps/native/app/android/settings.gradle (1)
Path references to node_modules are inconsistent across Gradle files
The verification revealed inconsistencies in the path depth when referencing node_modules:
settings.gradle
uses 4 levels:../../../../node_modules/
build.gradle
uses 5 levels:../../../../../node_modules/
app/build.gradle
mixes both 4 and 5 levels in comments and active codeThese inconsistencies should be normalized to ensure reliable builds.
🔗 Analysis chain
Line range hint
3-3
: Verify path consistency across the projectThe paths have been updated to reference the root node_modules:
- Native modules gradle script:
../../../../node_modules/@react-native-community/cli-platform-android/native_modules.gradle
- Gradle plugin:
../../../../node_modules/@react-native/gradle-plugin
These changes align with the PR objective to fix incorrect node_modules references.
Also applies to: 10-10
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that no other files still reference the app's node_modules rg -g '*.gradle' -g '*.properties' '../node_modules' apps/native/app/android/Length of output: 1856
codemagic.yaml (1)
222-222
: Consider documenting credential groups.To help other developers understand the CI/CD setup, consider adding documentation that explains:
- The purpose of different credential groups
- How to set them up in Codemagic
- Which environments use which credentials
Would you like me to help create a documentation template for the credential groups?
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
⛔ Files ignored due to path filters (1)
yarn.lock
is excluded by!**/yarn.lock
,!**/*.lock
📒 Files selected for processing (5)
apps/native/app/android/app/build.gradle
(2 hunks)apps/native/app/android/build.gradle
(1 hunks)apps/native/app/android/settings.gradle
(1 hunks)apps/native/app/package.json
(1 hunks)codemagic.yaml
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- apps/native/app/package.json
🧰 Additional context used
📓 Path-based instructions (3)
apps/native/app/android/build.gradle (1)
Pattern apps/**/*
: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
apps/native/app/android/app/build.gradle (1)
Pattern apps/**/*
: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
apps/native/app/android/settings.gradle (1)
Pattern apps/**/*
: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
🔇 Additional comments (4)
apps/native/app/android/settings.gradle (1)
6-7
: Verify clipboard functionality after module removal
The removal of react-native-clipboard module might affect clipboard operations in the app.
apps/native/app/android/build.gradle (1)
36-36
: Path updates align with node_modules restructuring
The repository URLs have been correctly updated to reference the root node_modules:
- React Native:
$rootDir/../../../../../node_modules/react-native/android
- JSC Android:
$rootDir/../../../../../node_modules/jsc-android/dist
These changes maintain consistency with the updated project structure.
Also applies to: 40-40
apps/native/app/android/app/build.gradle (1)
20-24
: Path updates align with node_modules restructuring
The React Native configuration paths have been correctly updated to reference the root node_modules:
- reactNativeDir:
../../../../../node_modules/react-native
- codegenDir:
../../../../../node_modules/@react-native/codegen
- cliFile:
../../../../../node_modules/react-native/cli.js
These changes maintain consistency with the updated project structure.
codemagic.yaml (1)
222-222
: LGTM! Please verify the credential group exists.
The change from firebase_credentials
to firebase_credentials_dev
improves environment isolation between prod and dev builds. However, please ensure that:
- The
firebase_credentials_dev
group exists in Codemagic - The group contains the correct Firebase configuration for the dev environment
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.
LGTM 👍
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #17211 +/- ##
==========================================
- Coverage 35.76% 35.76% -0.01%
==========================================
Files 6931 6931
Lines 147977 147977
Branches 42173 42173
==========================================
- Hits 52931 52930 -1
- Misses 95046 95047 +1 Flags with carried forward coverage won't be shown. Click here to find out more. see 1 file with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
Datadog ReportAll test runs ✅ 98 Total Test Services: 0 Failed, 90 Passed Test ServicesThis report shows up to 10 services
🔻 Code Coverage Decreases vs Default Branch (1)
|
What
Checklist:
Summary by CodeRabbit
New Features
react-native-clipboard
module from the project configuration.Bug Fixes
react-native-reanimated
dependency.Chores