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

feat(doctor): adjust healthchecks based on where command was ran #2049

Merged
merged 4 commits into from
Sep 20, 2023

Conversation

szymonrybczak
Copy link
Collaborator

@szymonrybczak szymonrybczak commented Aug 8, 2023

Summary:

Update doctor command to detect if command was ran in React Native app, if yes we can show the project specific checks.

Test plan

  1. Clone the repository and do all the required steps from the Contributing guide
  2. Run this command outside of React Native project, and you should see only checks related to setup.
node /path/to/react-native-cli/packages/cli/build/bin.js doctor
Common
 ✓ Node.js - Required to execute JavaScript code
 ✓ yarn - Required to install NPM dependencies
 ✓ Watchman - Used for watching changes in the filesystem when in development mode

Android
 ✓ Adb - Required to verify if the android device is attached correctly
 ✓ JDK - Required to compile Java code
 ✓ Android Studio - Required for building and installing your app on Android
 ✓ ANDROID_HOME - Environment variable that points to your Android SDK installation

iOS
 ✓ Xcode - Required for building and installing your app on iOS
 ✓ Ruby
 ✓ CocoaPods - Required for installing iOS dependencies
 ✓ ios-deploy - Required for installing your app on a physical device with the CLI

Errors:   0
Warnings: 0
  1. Run this command inside of React Native project, and you should see checks related to setup and to project.
node /path/to/react-native-cli/packages/cli/build/bin.js doctor
Common
 ✓ Node.js - Required to execute JavaScript code
 ✓ yarn - Required to install NPM dependencies
 ✓ Watchman - Used for watching changes in the filesystem when in development mode
 ✓ Metro - Required for bundling the JavaScript code

Android
 ✓ Adb - Required to verify if the android device is attached correctly
 ✓ JDK - Required to compile Java code
 ✓ Android Studio - Required for building and installing your app on Android
 ✓ ANDROID_HOME - Environment variable that points to your Android SDK installation
 ✓ Android SDK - Required for building and installing your app on Android

iOS
 ✓ Xcode - Required for building and installing your app on iOS
 ✓ Ruby - Required for installing iOS dependencies
 ✓ CocoaPods - Required for installing iOS dependencies
 ✓ ios-deploy - Required for installing your app on a physical device with the CLI
 ✓ .xcode.env - File to customize Xcode environment

Errors:   0
Warnings: 0

Checklist

  • Documentation is up to date to reflect these changes.
  • Follows commit message convention described in CONTRIBUTING.md

@thymikee
Copy link
Member

Initially we thought about using doctor as a tool to verify that the environment is set up correctly to init an app. I guess that use case could be moved to init anyway

@thymikee
Copy link
Member

cc @arushikesarwani94 wdyt?

@arushikesarwani94
Copy link
Contributor

ATM doctor is definitely a tool to verify that the environment is set up correctly to init an app. Not yet moved completely to init. This is also majorly used in issues reported in RN repo as a recommendation to run doctor and paste it's output to quickly spot something wrong with the set-up. The plan was also to move RN packages like cli-doctor out of CLI to RN repo but nothing concrete has happened on that front.
cc : @cortinico

@szymonrybczak
Copy link
Collaborator Author

szymonrybczak commented Aug 14, 2023

What we should take into account is that right now we have few parts of doctor that depends strongly on a project. To name few: .xcode.env and starting Metro. I think we should somehow maybe check if we're outside of project, if yes do not show project-specific options.

@cortinico
Copy link
Member

I think we should somehow maybe check if we're outside of project, if yes do not show project-specific options.

IMHO This should be the preferred approach.

that it doesn't make sense to allow running this command outside of project.

My 2c: It still makes sense to run outside of a project.
For example, say that you want to check if your machine is correctly configured to run a React Native project. Some checks are project specific, but others are not (i.e. you will need an Android/iOS SDK at some point).

@szymonrybczak
Copy link
Collaborator Author

@cortinico make sense, so I will adjust this PR to check if the command was ran outside React Native project if yes I will hide the project specific checks. 👍

@szymonrybczak szymonrybczak force-pushed the fix/move-doctor-to-project-commands branch 2 times, most recently from a9c05a1 to 1c79acc Compare August 14, 2023 14:01
@szymonrybczak szymonrybczak changed the title fix(breaking): move doctor command to project commands feat(doctor): adjust healthchecks based on where command was ran Aug 14, 2023
@szymonrybczak szymonrybczak force-pushed the fix/move-doctor-to-project-commands branch from 1c79acc to 25f8d52 Compare August 14, 2023 14:07
@adamTrz adamTrz removed the bugfix label Aug 19, 2023
@szymonrybczak
Copy link
Collaborator Author

szymonrybczak commented Sep 18, 2023

After final testing, the loadConfig function also returns config when running command e.g. inside CLI - when calling config.reactNativePath, we're sure that command has been run in React Native project.

@thymikee thymikee merged commit 4fac063 into main Sep 20, 2023
@thymikee thymikee deleted the fix/move-doctor-to-project-commands branch September 20, 2023 15:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants