Skip to content

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

Merged
thymikee merged 4 commits into
mainfrom
fix/move-doctor-to-project-commands
Sep 20, 2023
Merged

feat(doctor): adjust healthchecks based on where command was ran#2049
thymikee merged 4 commits into
mainfrom
fix/move-doctor-to-project-commands

Conversation

@szymonrybczak

@szymonrybczak szymonrybczak commented Aug 8, 2023

Copy link
Copy Markdown
Collaborator

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
Copy Markdown
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
Copy Markdown
Member

cc @arushikesarwani94 wdyt?

@arushikesarwani94

Copy link
Copy Markdown
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

szymonrybczak commented Aug 14, 2023

Copy link
Copy Markdown
Collaborator Author

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
Copy Markdown
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
Copy Markdown
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

szymonrybczak commented Sep 18, 2023

Copy link
Copy Markdown
Collaborator Author

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