Skip to content
This repository has been archived by the owner on Apr 20, 2023. It is now read-only.

chore: add node badge to README #138

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

jhonatantft
Copy link

Related issue: #22

What changes this code is introducing?

  • Adds a badge that tells the user the lowest supported version of node
  • Specified the version in the engines property in package.json

@facebook-github-bot
Copy link
Contributor

Hi @jhonatantft!

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 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 to sign the corporate CLA.

If you have received this in error or have any questions, please contact us at [email protected]. Thanks!

@facebook-github-bot
Copy link
Contributor

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

1 similar comment
@facebook-github-bot
Copy link
Contributor

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

@jhonatantft
Copy link
Author

@pestevez Could you review it please?

@pestevez
Copy link
Contributor

pestevez commented Feb 9, 2021

Apologies for the delayed response, @jhonatantft. The changes look good. Would you mind sharing how you landed at that specific version being the minimum one supported?

@jhonatantft
Copy link
Author

@pestevez Thanks for the answer. I managed to get to that specific version by doing tests with each Node version. In the older version than this, I received several errors and was unable to use the application properly.

@pestevez
Copy link
Contributor

I tried with version 8.11.3 (one lower than the one proposed) and it worked, but it didn't work with version 8.0.0. I am fine with keeping the one you have, since anyway the LTS version of node as of today is 14.15.5.

Can you please rebase, run npm install, AND include the package-lock.json file in your PR? Thanks again.

@pestevez pestevez self-assigned this Feb 10, 2021
@jhonatantft jhonatantft force-pushed the feat/node-version-instruction branch from 73c6bec to 383c175 Compare February 10, 2021 16:41
@jhonatantft
Copy link
Author

Done! @pestevez

@pestevez pestevez self-requested a review February 10, 2021 17:27
Copy link
Contributor

@pestevez pestevez left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution @jhonatantft! Feel free to merge it.

@pestevez
Copy link
Contributor

You'll have to rebase again since we merged another PR that modified the lock file. I can take another look after you repeat the same steps described above.

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.

3 participants