Skip to content

Fix typo in installation instructions#7933

Closed
mgmarlow wants to merge 1 commit intofacebook:masterfrom
mgmarlow:patch-1
Closed

Fix typo in installation instructions#7933
mgmarlow wants to merge 1 commit intofacebook:masterfrom
mgmarlow:patch-1

Conversation

@mgmarlow
Copy link
Copy Markdown
Contributor

Installation instructions for babel + NPM instructed installing the incorrect babel plugin.

@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!

@goodmind
Copy link
Copy Markdown
Contributor

@mgmarlow I thought there already was ton of PRs fixing this, but each next PR finds old babel plugin... Can you sign CLA?

@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!

@mroch
Copy link
Copy Markdown
Contributor

mroch commented Jul 17, 2019

thanks!

@goodmind: there are at least 3 other PRs doing various conflicting things, in various needs of rebasing.

#6396 makes a similar change to a different part of the docs, which seems to have already been fixed
#6952 is similar to this PR, but it also updates babel-cli to @babel/cli and replaces some other references, which I think mostly have already been fixed by other PRs.
#7049 also switches to @babel/cli but additionally installs @babel/core. I have no idea if this is necessary.

since this one is current, it seems like the only one that can be merged without conflicts. I think we should probably update to @babel/cli and maybe add @babel/core. since we don't use this workflow internally, maybe you guys can help test that?

@mgmarlow
Copy link
Copy Markdown
Contributor Author

@mroch sure I'll take a look. I set my project up using rollup so I didn't need the CLI but I can run through a similar flow with those tools.

@mgmarlow
Copy link
Copy Markdown
Contributor Author

@mroch You're 100% correct, you also need @babel/core when running @babel/cli. I've updated the docs accordingly.

@mroch
Copy link
Copy Markdown
Contributor

mroch commented Jul 17, 2019

awesome, thank you! (and also @KidkArolis and @jeffvandyke for #7049)

Copy link
Copy Markdown
Contributor

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

Choose a reason for hiding this comment

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

@mroch has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Copy Markdown
Contributor

@mroch merged this pull request in 955e926.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants