Skip to content

Conversation

@danthareja
Copy link
Contributor

Closes #217

@ghost
Copy link

ghost commented Jul 26, 2016

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 - and if you have received this in error or have any questions, please drop us a line at [email protected]. Thanks!

@ghost
Copy link

ghost commented Jul 26, 2016

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

@ghost ghost added the CLA Signed label Jul 26, 2016
Copy link
Contributor

@hnordt hnordt Jul 26, 2016

Choose a reason for hiding this comment

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

The generated project only includes build dependencies.

Because npm install --save <library-name> is considered a build dependency too the above sentence looks confusing to me.

Also I consider React and ReactDOM as project-specific dependencies too.

I would like to suggest a slighty change:

The generated project includes React and ReactDOM as dependencies and Create React App as a development dependency. Other dependencies (Redux, React Router, etc.) can be installed with npm, without having to modify any existing configuration.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point about React and ReactDOM also being dependencies. I like the explicitness of your copy, thanks for the suggestion!

@danthareja danthareja force-pushed the docs/install-dependency branch from 504f0c3 to 6ae333e Compare July 26, 2016 21:44
@ghost ghost added the CLA Signed label Jul 26, 2016
Copy link
Contributor

@gaearon gaearon Jul 27, 2016

Choose a reason for hiding this comment

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

Let’s change it to:

The generated project includes React and ReactDOM as dependencies. It also includes a set of scripts used by Create React App as a development dependency.

Copy link
Contributor

Choose a reason for hiding this comment

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

Let’s avoid passive and trim the sentence down :

You may install other dependencies (for example, React Router) with npm:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good suggestions - thanks!

@danthareja danthareja force-pushed the docs/install-dependency branch from 6ae333e to 651a254 Compare July 27, 2016 16:25
@ghost ghost added the CLA Signed label Jul 27, 2016
@danthareja
Copy link
Contributor Author

Bump @gaearon

@mxstbr
Copy link
Contributor

mxstbr commented Jul 27, 2016

This looks good, thanks @danthareja!

@mxstbr mxstbr merged commit f97b1bb into facebook:master Jul 27, 2016
ianschmitz pushed a commit to ianschmitz/create-react-app that referenced this pull request Jan 19, 2018
Fixed problem with tsconfig.json baseUrl and paths
@lock lock bot locked and limited conversation to collaborators Jan 22, 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.

4 participants