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

Update issue template to include the --info flag #3797

Closed
wants to merge 3 commits into from

Conversation

bondz
Copy link
Contributor

@bondz bondz commented Jan 15, 2018

Document using the --info flag exposed by #3408 when reporting issues in CRA.

For platforms that have a built in command to pipe from terminal to clipboard (OSX and Windows), the suggested command copies the result to the clipboard automatically. Although, it also never prints the result to terminal for the user to see first, which might be a bummer, suggestions welcome.

For Linux users, there are different ways to pipe the result of a command to terminal, I'll leave it up to the user to use one.

@tabrindle
Copy link
Contributor

Thanks @bondz. Was going to get to this after the holiday.

Do we need to keep the old commands?

Also envinfo can auto copy to clipboard built in via option as well get browser versions if we want to add this later on.

@tabrindle
Copy link
Contributor

Also for the npx invocation of the command, it might be faster to use envinfo directly?

@bondz
Copy link
Contributor Author

bondz commented Jan 15, 2018

@tabrindle The reason I kept the old commands is based on the assumption that CRA does not advertize updating the CRA module itself, so, some users might not have it available.

Although, we could recommend that users are on that version like react-native does.

For the npx command, I opted to use CRA because the configuration info for CRA might diverge from that of envinfo in the future. That way, we wouldn't have to modify the issue template each time. It might be unnecessary if we recommend users be on a version of CRA with --info support.

The clipboard helper is a convenience especially for a user creating an issue and its explicit. Not sure if the default info command should automatically copy to clipboard, but would be nice since it works cross-platform on envinfo. Getting browsers would be a nice addition 👍

2. `npm -v`:
3. `yarn --version` (if you use Yarn):
4. `npm ls react-scripts` (if you haven’t ejected):
If you have create-react-app >= 1.5.0 installed, the `--info` flag generates information about your environment.
Copy link
Contributor

Choose a reason for hiding this comment

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

I feel like since we advertise npx now, this is actually not super likely.

Run the command specific to your operating system in your react app's folder
Windows: `create-react-app --info | clip`
OSX: `create-react-app --info | pbcopy`
Copy link
Contributor

@Timer Timer Jan 16, 2018

Choose a reason for hiding this comment

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

nit (sorry): macOS please

@gaearon
Copy link
Contributor

gaearon commented Jan 20, 2018

Could you rebase please? I made some changes to the template that don't merge cleanly.

@bondz
Copy link
Contributor Author

bondz commented Jan 20, 2018

Done. Looks like its messed up now though. sigh.

@bondz
Copy link
Contributor Author

bondz commented Jan 20, 2018

Fixed.

macOS: `create-react-app --info | pbcopy`
Linux: `create-react-app --info`

If you get an error, you can use `npx` or `yarn` by running one of the commands below in terminal.
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn’t these instructions come first? Users who follow our README will get an error.

@bondz
Copy link
Contributor Author

bondz commented Jan 21, 2018

I tested on macOS and Windows and it works. It errors on Linux without the xsel binary available.

I also tested it works on an ejected version of CRA. The react-scripts section of packages is missing.

#3859 should be available before this, otherwise, a version of Android Studio and XCode would be printed.

It took ~20sec to run on my Mac, even longer on Windows. There is no way to make npx use yarn yet see yarnpkg/yarn#3937

@bondz
Copy link
Contributor Author

bondz commented Apr 29, 2018

Closing in favour of #4375

@bondz bondz closed this Apr 29, 2018
@bondz bondz deleted the patch-1 branch October 2, 2018 02:11
@lock lock bot locked and limited conversation to collaborators Jan 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants