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

Add minimum node version to engine field in package.json #4790

Merged
merged 3 commits into from May 20, 2020
Merged

Add minimum node version to engine field in package.json #4790

merged 3 commits into from May 20, 2020

Conversation

rgranger
Copy link
Contributor

Which issue, if any, is this issue related to?

#4787

Is there anything in the PR that needs further explanation?

e.g. "No, it's self-explanatory."

Copy link
Member

@jeddy3 jeddy3 left a comment

Choose a reason for hiding this comment

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

@rgranger Thanks! I've requested a change.

You'll also need to run npm run format to resolve the lint warning.


As an aside, once we roll out stylelint/npm-package-json-lint-config across the stylelint repositories they'll all have engine fields as the config requires it.

README.md Outdated
@@ -19,6 +19,8 @@ It's mighty as it:
- is **unopinionated** so that you can customize it to your exact needs
- has a **growing community** and is used by [Facebook](https://code.facebook.com/posts/879890885467584/improving-css-quality-at-facebook-and-beyond/), [GitHub](https://github.com/primer/stylelint-config-primer) and [WordPress](https://github.com/ntwb/stylelint-config-wordpress/)

Minimum NodeJs version supported : v10.13.0
Copy link
Member

Choose a reason for hiding this comment

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

We can remove this line as using the engine field in the package.json will suffice. It's the standardised way of communicating a package's compatibility and will produce a warning on the CLI when npm i is used, e.g.:

npm WARN notsup Unsupported engine for stylelint@13.4.0: wanted: {"node":">=10.13.0"} (current: {"node":"8.17.0","npm":"6.13.4"})

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, line has been removed from the readme

@jeddy3
Copy link
Member

jeddy3 commented May 19, 2020

@rgranger Thanks for making the change.

You'll need to run npm run format to resolve the lint warning.

It'll likely reorder the engine property in the package.json to be consistent with stylelint's other repositories.

@jeddy3 jeddy3 changed the title chore(doc) : add information about the minimum node version supported Add minimum node version to engine field in package.json May 19, 2020
@rgranger
Copy link
Contributor Author

done

Copy link
Member

@m-allanson m-allanson left a comment

Choose a reason for hiding this comment

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

Thank you @rgranger. This looks good to me 👍

Copy link
Member

@jeddy3 jeddy3 left a comment

Choose a reason for hiding this comment

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

Thanks!

Looks good to me.

@jeddy3 jeddy3 merged commit cc8efb1 into stylelint:master May 20, 2020
@jeddy3
Copy link
Member

jeddy3 commented May 20, 2020

Changelog:

  • Fixed: specify minimum node version in package.json's engine field (#4790).

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

Successfully merging this pull request may close these issues.

None yet

3 participants