Skip to content

Update minimum NodeJS and NPM versions required #140

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

Merged
merged 1 commit into from
Nov 15, 2016
Merged

Update minimum NodeJS and NPM versions required #140

merged 1 commit into from
Nov 15, 2016

Conversation

ntwb
Copy link
Contributor

@ntwb ntwb commented Nov 15, 2016

This module requires postcss-load-config which in turn requires postcss-load-plugins and postcss-load-options, all three of these repos have a NodeJS & NPM minimum requirement:

• "engines": { "node": ">=0.12", "npm": ">=3" },`

https://github.com/michael-ciniawsky/postcss-load-config/blob/master/package.json#L5
https://github.com/michael-ciniawsky/postcss-load-plugins/blob/master/package.json#L5
https://github.com/michael-ciniawsky/postcss-load-options/blob/master/package.json#L5

To prevent warnings when using this module this repo should also follow the same min req.

@ai
Copy link
Contributor

ai commented Nov 15, 2016

@ntwb why we should add engines here and not to postcss-load-config?

@michael-ciniawsky
Copy link
Member

michael-ciniawsky commented Nov 15, 2016

@ai Doesn't hurt to provide metadata about node and npm version, I would say merge it 😛

@ntwb Doesnpm install --save|--save-dev postcss-loader currently display any warnings, because of missing pkg.engines prop?

@ntwb
Copy link
Contributor Author

ntwb commented Nov 15, 2016

@ai, postcss-load-config already has engines defined: https://github.com/michael-ciniawsky/postcss-load-config/blob/master/package.json#L5

Here's the details of Travis CI job I noticed this in: https://travis-ci.org/stylelint/stylelint-demo/jobs/175820550#L172

@ai
Copy link
Contributor

ai commented Nov 15, 2016

@ntwb so, do we need a engines in postcss-loader if it is already in postcss-load-config?

@ntwb
Copy link
Contributor Author

ntwb commented Nov 15, 2016

@ntwb so, do we need a engines in postcss-loader if it is already in postcss-load-config?

Yes, which is what this PR fixes 😄

@ai
Copy link
Contributor

ai commented Nov 15, 2016

@ntwb and why we need it if it is already in postcss-load-config?

@ntwb
Copy link
Contributor Author

ntwb commented Nov 15, 2016

If a repo you depend upon requires NodeJS v7.x, then this repo should also state that the minimum requirement is NodeJS v7.x

In this case though postcss-load-config has a minimum NPM requirement for NPM v3.x, so that same minimum requirement should be included in postcss-loader

Edit: You cannot use postcss-loader if you use NPM 2.x, you must have NPM 3.x

@ai
Copy link
Contributor

ai commented Nov 15, 2016

I mean when benefits user will get? npm warning will be the same?

@ntwb
Copy link
Contributor Author

ntwb commented Nov 15, 2016

They will no longer get the NPM warning

@ai ai merged commit 16292d5 into webpack-contrib:master Nov 15, 2016
@ntwb ntwb deleted the patch-1 branch November 16, 2016 00:21
@ntwb
Copy link
Contributor Author

ntwb commented Nov 16, 2016

Thanks :)

@TrySound TrySound mentioned this pull request Dec 19, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants