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

Move expo-doctor over from expo-cli #25503

Merged
merged 14 commits into from
Nov 30, 2023
Merged

Conversation

keith-kurak
Copy link
Contributor

Why

Add expo-doctor to this repository for future maintenance

How

Questions

  • I'm not sure what, if anything needs to be added to CI jobs, since Doctor only has unit tests currently
  • Kind of guessing as to what tsconfig.json should look like, mostly copied @expo/cli with a few added package-specific flags

Test Plan

  • Test locally
  • see if CI passes

@keith-kurak keith-kurak requested review from byCedric and removed request for alanjhughes November 21, 2023 18:12
@expo-bot expo-bot added the bot: passed checks ExpoBot has nothing to complain about label Nov 21, 2023
Copy link
Member

@byCedric byCedric left a comment

Choose a reason for hiding this comment

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

Two minor comments, but looking good overal!

packages/expo-doctor/CHANGELOG.md Outdated Show resolved Hide resolved
packages/expo-doctor/src/__tests__/asMock.ts Outdated Show resolved Hide resolved
Copy link
Member

@byCedric byCedric left a comment

Choose a reason for hiding this comment

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

This is just a suggestion we could do, see #25424 (comment) for more info.

packages/expo-doctor/jest.config.js Outdated Show resolved Hide resolved
keith-kurak and others added 3 commits November 22, 2023 09:51
Co-authored-by: Cedric van Putten <me@bycedric.com>
Co-authored-by: Cedric van Putten <me@bycedric.com>
Copy link
Member

@byCedric byCedric left a comment

Choose a reason for hiding this comment

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

Two small nits, but looking good! Thanks for adding

packages/expo-doctor/README.md Outdated Show resolved Hide resolved
packages/expo-doctor/package.json Outdated Show resolved Hide resolved
keith-kurak and others added 2 commits November 29, 2023 16:05
Co-authored-by: Cedric van Putten <me@bycedric.com>
Co-authored-by: Cedric van Putten <me@bycedric.com>
@keith-kurak keith-kurak merged commit d8897a4 into main Nov 30, 2023
15 checks passed
@keith-kurak keith-kurak deleted the keith/move-doctor-to-expo-expo branch November 30, 2023 16:10
onizam95 pushed a commit to onizam95/expo-av-drm that referenced this pull request Jan 15, 2024
# Why
Add expo-doctor to this repository for future maintenance

# How

- Copied over expo-doctor folder from expo-cli/packages
- Adapted changes from expo#25422 to switch
scripts to `expo-module` equivalents
- updated unit tests to properly mock `spawnAsync`
- Fixed linter warnings

# Questions
- I'm not sure what, if anything needs to be added to CI jobs, since
Doctor only has unit tests currently
- Kind of guessing as to what **tsconfig.json** should look like, mostly
copied @expo/cli with a few added package-specific flags

# Test Plan
-  [x] Test locally
-  [ ] see if CI passes

---------

Co-authored-by: Cedric van Putten <me@bycedric.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bot: fingerprint changed bot: passed checks ExpoBot has nothing to complain about
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants