Skip to content

Use installing package manager in README - #7687

Merged
mrmckeb merged 3 commits into
react:masterfrom
ashr81:master
Oct 2, 2019
Merged

Use installing package manager in README#7687
mrmckeb merged 3 commits into
react:masterfrom
ashr81:master

Conversation

@ashr81

@ashr81ashr81 commented Sep 16, 2019

Copy link
Copy Markdown
Contributor

Fixes#7411

Replaces npm commands to yarn commands if the user uses yarn

@facebook-github-bot

facebook-github-bot commented Sep 16, 2019

Copy link
Copy Markdown

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

facebook-github-bot commented Sep 16, 2019

Copy link
Copy Markdown

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

iansu
iansu suggested changes Sep 26, 2019
View reviewed changes
Comment threadpackages/react-scripts/scripts/init.js Outdated
// modifies README.md commands based on user used package manager.
if (useYarn) {
try {
const data = fs.readFileSync(path.join(appPath, 'README.md'), 'utf8');

@iansuiansuSep 26, 2019

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.

Rename data to readme and modify that directly instead of creating another formatted variable.

@ashr81ashr81Sep 27, 2019

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

@iansu I renamed variable to the readme and modifying directly. I used variable naming reference from the same file.

@iansuiansu added this to the 3.1.3 milestone Sep 26, 2019
@iansu

iansu commented Sep 26, 2019

Copy link
Copy Markdown
Contributor

This is great. Just one small change from me and then I think we can merge it. @mrmckeb has been doing some work on this part of the code. Any concerns?

@ashr81

ashr81 commented Sep 27, 2019

Copy link
Copy Markdown
ContributorAuthor

@iansu I saw the PR raised by him. I have no concerns. Let me know if you want something to be done from the end.

@iansuiansu self-assigned this Sep 27, 2019
@ashr81
ashr81 requested a review from iansuSeptember 28, 2019 06:10
@ashr81

ashr81 commented Oct 1, 2019

Copy link
Copy Markdown
ContributorAuthor

@iansu can you please review this PR. Is the failing test cases stopping this from getting merged? Or do you have any concerns on the PR?

@mrmckeb

mrmckeb commented Oct 1, 2019

Copy link
Copy Markdown
Contributor

I think this won't cause any issues for now, I'll need to do a rebase before we move forward on templates anyway.

@mrmckebmrmckeb modified the milestones: 3.1.3, 3.2Oct 1, 2019
@ashr81

ashr81 commented Oct 2, 2019

Copy link
Copy Markdown
ContributorAuthor

@mrmckeb cool thanks. If changing the base branch to feature/templates helps you, which has your template changes. Go ahead and change the base branch. I will resolve the conflicts if any arises.

@mrmckebmrmckeb closed this Oct 2, 2019
@mrmckeb

mrmckeb commented Oct 2, 2019

Copy link
Copy Markdown
Contributor

Restarting CI

@mrmckebmrmckeb reopened this Oct 2, 2019
@mrmckebmrmckeb changed the title updates readme based on package manager used by user.Use installing package manager in READMEOct 2, 2019
@mrmckeb
mrmckeb merged commit 6b8fa00 into react:masterOct 2, 2019
@mrmckeb

mrmckeb commented Oct 2, 2019

Copy link
Copy Markdown
Contributor

Thank you!

@iansuiansu removed this from the 3.3 milestone Oct 2, 2019
@iansuiansu added this to the 3.2 milestone Oct 2, 2019
@locklockBot locked and limited conversation to collaborators Oct 7, 2019
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@ianschmitzianschmitzAwaiting requested review from ianschmitz
@petetntpetetntAwaiting requested review from petetnt
@iansuiansuAwaiting requested review from iansu
1 more reviewer
@mrmckebmrmckebmrmckeb approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@iansuiansu

Labels

CLA Signedtag: enhancement

Projects

None yet

Milestone

3.2

Development

Successfully merging this pull request may close these issues.

Generated README containing npm examples is confusing, should be changed to yarn

4 participants

@ashr81@facebook-github-bot@iansu@mrmckeb