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

chore: adjust run instruction based on the user platform #1285

Conversation

Simek
Copy link
Member

@Simek Simek commented Oct 6, 2020

Summary:

This PR adjust the run instructions printed at the end of init command run which now should show only valid targets for the user platform. I have also updated the links for the desktop forks from MS to lead exactly to platform specific guides.

  • iOS and macOS instructions will be only visible on darwin
  • Windows instructions will be only visible on win32

Test Plan:

I was trying to test the init locally using Contributing guide, but I have experienced problems and issues after publishing react-native master locally.

One of many problems which leads to init termination:

error Couldn't find package "@typescript-eslint/experimental-utils@2.34.0" required by "@typescript-eslint/eslint-plugin@^2.25.0" on the "npm" registry.
info Visit https://yarnpkg.com/en/docs/cli/add for documentation about this command.
Error: Couldn't find package "eslint-plugin-flowtype@2.50.3" required by "@react-native-community/eslint-config@^1.1.0" on the "npm" registry.
    at MessageError.ExtendableBuiltin (/usr/local/Cellar/yarn/1.22.5/libexec/lib/cli.js:721:66)
    at new MessageError (/usr/local/Cellar/yarn/1.22.5/libexec/lib/cli.js:750:123)
    at PackageRequest.<anonymous> (/usr/local/Cellar/yarn/1.22.5/libexec/lib/cli.js:36539:17)
    at Generator.throw (<anonymous>)
    at step (/usr/local/Cellar/yarn/1.22.5/libexec/lib/cli.js:310:30)
    at /usr/local/Cellar/yarn/1.22.5/libexec/lib/cli.js:323:13
    at processTicksAndRejections (internal/process/task_queues.js:93:5)

Please let me know is there any other way to test the init command without publishing react-native locally.

@Simek
Copy link
Member Author

Simek commented Oct 6, 2020

@Simek Simek force-pushed the adjust-run-instuction-based-on-platform branch from c57240e to 5fb5632 Compare October 6, 2020 10:33
@thymikee
Copy link
Member

thymikee commented Oct 6, 2020

Mind sending some screenshots on how the end result looks now (at least on one platform)?

@Simek
Copy link
Member Author

Simek commented Oct 6, 2020

Preview

@thymikee It looks like I had to remove one indentation from the nested in conditionals instructions:

Screenshot 2020-10-06 at 13 32 04

@thymikee
Copy link
Member

thymikee commented Oct 6, 2020

Let's address this then

@Simek
Copy link
Member Author

Simek commented Oct 6, 2020

Preview after whitespace changes (macOS)

Screenshot 2020-10-06 at 13 40 15

@Simek
Copy link
Member Author

Simek commented Oct 6, 2020

Windows preview

Screenshot 2020-10-06 192332

@Simek
Copy link
Member Author

Simek commented Oct 6, 2020

@asklar Is it possible to create the short URLs for both platforms, leading to Getting Started pages?

For example:

  • Windows: https://aka.ms/ReactNativeWin, ReactNativeGuideWindows (ReactNativeWindows is taken by the GitHub repository)
  • macOS: https://aka.ms/ReactNativeMacOS, ReactNativeGuideMacOS

@asklar
Copy link
Contributor

asklar commented Oct 6, 2020

@Simek I've created ReactNativeMacOS, ReactNativeGuideWindows and ReactNativeGuideMacOS

@Simek Simek force-pushed the adjust-run-instuction-based-on-platform branch from 4451e83 to 5db47f3 Compare October 7, 2020 13:58
@Simek
Copy link
Member Author

Simek commented Oct 7, 2020

@thymikee I have changed the color of Windows instructions header to cyan to avoid the black font issue in PS on Windows. macOS header can still use magenta, because there are no issues with rendering this color in Terminal.

@thymikee thymikee merged commit 5ec3fcf into react-native-community:master Oct 7, 2020
@thymikee
Copy link
Member

thymikee commented Oct 7, 2020

Thank you @Simek 🚀 !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants