Skip to content

cli: upgrade envinfo for new features in react-native info#19331

Closed
tabrindle wants to merge 2 commits into
react:masterfrom
tabrindle:update-envinfo-dep
Closed

cli: upgrade envinfo for new features in react-native info#19331
tabrindle wants to merge 2 commits into
react:masterfrom
tabrindle:update-envinfo-dep

Conversation

@tabrindle

Copy link
Copy Markdown
Contributor

Motivation

envinfo has done a good job reporting issues in the issue template so far, and I've done a lot of work between version 3.x and 5.x that react-native could benefit from. This adds:

  • better information organization, including versions and paths
  • Platform/CPU/RAM
  • Android and iOS SDK version detection
  • npm package globbing (select all babel* packages
  • global npm packages (with globbing)

envinfo also can report IDE versions, other binaries, languages and browsers if needed, and in different formats. Take a look here if interested: https://github.com/tabrindle/envinfo

Test Plan

  • run react-native info // standard info
  • run react-native info --packages // all packages in package.json
  • run react-native info --packages jest,eslint,babel-polyfill // specified packages
  • run react-native info --packages *babel* // globbed packages

Sample standard output:

  System:
    OS: macOS High Sierra 10.13
    CPU: x64 Intel(R) Core(TM) i7-4870HQ CPU @ 2.50GHz
    Memory: 97.59 MB / 16.00 GB
    Shell: 5.4.2 - /usr/local/bin/zsh
  Binaries:
    Node: 8.11.0 - ~/.nvm/versions/node/v8.11.0/bin/node
    Yarn: 1.5.1 - ~/.yarn/bin/yarn
    npm: 5.6.0 - ~/.nvm/versions/node/v8.11.0/bin/npm
    Watchman: 4.9.0 - /usr/local/bin/watchman
  SDKs:
    iOS SDK:
      Platforms: iOS 11.0, macOS 10.13, tvOS 11.0, watchOS 4.0
    Android SDK:
      Build Tools: 27.0.3
      API Levels: 26
  IDEs:
    Android Studio: 3.0 AI-171.4443003
    Xcode: 9.0/9A235 - /usr/bin/xcodebuild
  npmPackages:
    react: 16.3.2 => 16.3.2
    react-native: 0.55.0 => 0.55.0
  npmGlobalPackages:
    create-react-native-app: 1.0.0
    react-native-cli: 2.0.1

Related PRs

#14428 - original inclusion of react-native info

Release Notes

[CLI] [ENHANCEMENT] [local-cli/info/info.js] - add more info to react-native info cli command, like global npm packages, binary paths, and SDK versions

@tabrindle
tabrindle requested a review from hramos as a code owner May 17, 2018 15:19
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label May 17, 2018
@pull-bot

pull-bot commented May 17, 2018

Copy link
Copy Markdown
Warnings
⚠️

🔒 package.json - Changes were made to package.json. This will require a manual import by a Facebook employee.

Generated by 🚫 dangerJS

@hramos

hramos commented May 17, 2018

Copy link
Copy Markdown
Contributor

Adding envinfo to the CLI has had a huge impact on the quality of issues that are submitted to the repo. Thanks for setting it up!

@hramos hramos left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

PR looks great, but before we merge we should make sure react-native-bot has been updated to handle the new envinfo output. Otherwise it might incorrectly flag issues that do contain the information required under "Environment".

Comment thread local-cli/info/info.js Outdated
envinfo
.run(
{
System: ['OS', 'CPU', 'Memory', 'Shell'],

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

react-native-bot is set up to look for the old envinfo snippet (see lines 112 - 124). Since we have control over how this data is printed out when it's done within a RN project, we might as well print out some unique string that we can easily grep for.

Currently that is "Packages: (wanted => installed)" but that's fragile. I don't have any suggestion at the moment, other than it should be a string that is unlikely to appear elsewhere in a GitHub issue.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

oh wow, good catch.

We can add a custom title above the output in a .then block to look like this:

React Native Environment Info:
  System:
    OS: macOS High Sierra 10.13
    CPU: x64 Intel(R) Core(TM) i7-4870HQ CPU @ 2.50GHz

Then look for that?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yep that would work!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ill make the changes here - do you want to make the changes to the bot?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yep I can do that.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Done!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

OK, I decided this is common enough of an ask to include this in envinfo@5.7.0. This means the --clipboard option will work correctly.

Updating and adding the title option.

@hramos hramos self-assigned this May 17, 2018
@facebook-github-bot facebook-github-bot added the Import Started This pull request has been imported. This does not imply the PR has been approved. label May 17, 2018

@facebook-github-bot facebook-github-bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@hramos is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@facebook-github-bot facebook-github-bot added Import Failed and removed Import Started This pull request has been imported. This does not imply the PR has been approved. labels May 18, 2018
@facebook-github-bot

Copy link
Copy Markdown
Contributor

I tried to merge this pull request into the Facebook internal repo but some checks failed. To unblock yourself please check the following: Does this pull request pass all open source tests on GitHub? If not please fix those. Does the code still apply cleanly on top of GitHub master? If not can please rebase. In all other cases this means some internal test failed, for example a part of a fb app won't work with this pull request. I've added the Import Failed label to this pull request so it is easy for someone at fb to find the pull request and check what failed. If you don't see anyone comment in a few days feel free to comment mentioning one of the core contributors to the project so they get a notification.

@tabrindle

Copy link
Copy Markdown
Contributor Author

@hramos do you want me to rebase or leave it as is?

@facebook-github-bot

Copy link
Copy Markdown
Contributor

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need the corporate CLA signed.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

@facebook-github-bot

Copy link
Copy Markdown
Contributor

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@hramos

hramos commented May 18, 2018

Copy link
Copy Markdown
Contributor

I'll tackle it

@hramos

hramos commented May 18, 2018

Copy link
Copy Markdown
Contributor

It's landing now. I had to update our offline yarn cache to account for the updated envinfo package.

@tabrindle

tabrindle commented May 18, 2018 via email

Copy link
Copy Markdown
Contributor Author

@tabrindle

Copy link
Copy Markdown
Contributor Author

@hramos How come this is still open?

@hramos

hramos commented May 29, 2018

Copy link
Copy Markdown
Contributor

@tabrindle anything that touches package.json will inevitable take some time to land properly due to the manual work it requires, and in this case, tests unrelated to this change were failing internally. I have a couple dozen diffs in flight at any time and restarting each one can take up to half a day to rebase and run through tests, again. It's part of the pain we have to deal with as long as GitHub is not the source of truth for this repo. I'm working on it.

@tabrindle

Copy link
Copy Markdown
Contributor Author

Got it! Sorry for the bother. Let me know if there is anything I can do to help!

grabbou pushed a commit to react-native-community/cli that referenced this pull request Sep 26, 2018
Summary:
envinfo has done a good job reporting issues in the issue template so far, and I've done a lot of work between version 3.x and 5.x that react-native could benefit from. This adds:
- better information organization, including versions and paths
- Platform/CPU/RAM
- Android and iOS SDK version detection
- npm package globbing (select all babel* packages
- global npm packages (with globbing)

envinfo also can report IDE versions, other binaries, languages and browsers if needed, and in different formats. Take a look here if interested: https://github.com/tabrindle/envinfo

- run `react-native info` // standard info
- run `react-native info --packages` // all packages in package.json
- run `react-native info --packages jest,eslint,babel-polyfill` // specified packages
- run `react-native info --packages *babel*` // globbed packages

Sample standard output:
```
  System:
    OS: macOS High Sierra 10.13
    CPU: x64 Intel(R) Core(TM) i7-4870HQ CPU @ 2.50GHz
    Memory: 97.59 MB / 16.00 GB
    Shell: 5.4.2 - /usr/local/bin/zsh
  Binaries:
    Node: 8.11.0 - ~/.nvm/versions/node/v8.11.0/bin/node
    Yarn: 1.5.1 - ~/.yarn/bin/yarn
    npm: 5.6.0 - ~/.nvm/versions/node/v8.11.0/bin/npm
    Watchman: 4.9.0 - /usr/local/bin/watchman
  SDKs:
    iOS SDK:
      Platforms: iOS 11.0, macOS 10.13, tvOS 11.0, watchOS 4.0
    Android SDK:
      Build Tools: 27.0.3
      API Levels: 26
  IDEs:
    Android Studio: 3.0 AI-171.4443003
    Xcode: 9.0/9A235 - /usr/bin/xcodebuild
  npmPackages:
    react: 16.3.2 => 16.3.2
    react-native: 0.55.0 => 0.55.0
  npmGlobalPackages:
    create-react-native-app: 1.0.0
    react-native-cli: 2.0.1
```

react/react-native#14428 - original inclusion of `react-native info`

[CLI] [ENHANCEMENT] [local-cli/info/info.js] - add more info to react-native info cli command, like global npm packages, binary paths, and SDK versions
Closes react/react-native#19331

Differential Revision: D8049650

Pulled By: hramos

fbshipit-source-id: 35c677f369bcad1a014eb083b2ce60ba33fee0ea
@hramos hramos added the Merged This PR has been merged. label Mar 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants