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 nodejs supported version in package #307

Closed

Conversation

allevo
Copy link

@allevo allevo commented Jul 4, 2017

The .travis.yml describes the nodejs supported version.
This is not reflected in package.json.

(https://docs.npmjs.com/files/package.json#engines)

The `.travis.yml` describes the nodejs supported version.
This is not reflected in `package.json`.
@mcollina
Copy link
Member

mcollina commented Jul 4, 2017

I'm 👎 on this change. Why is it needed?

@allevo
Copy link
Author

allevo commented Jul 4, 2017

  • The first things I see in a npm module are the supported nodejs version in order to define the supported version in the my package.
  • Using npm the only way to define them is in package.json. And the test are run defining the nodejs versions (in .travis.yml). There's no reason to don't explicitly declare the supported nodejs version.
  • The main aim of this package is to achieve the back-compatibility through the stream versions.

That's why I thought it is useful.

@calvinmetcalf
Copy link
Contributor

@allevo one of the issues we face in older node is that some of the testing infrastructure doesn't work in older versions of node that readable-stream might, so doing this may break older versions of node that actually do work even if the tests don't

@allevo
Copy link
Author

allevo commented Jul 6, 2017

I use 0.8 version because I found it in .travis.yml file. tap works only with 0.8 or higher. This don't imply that the older version don't works. I pulled random the number.

But the other considerations are still valid.

Is there a known older nodejs version that works with redeable-stream?

@mcollina
Copy link
Member

mcollina commented Jul 9, 2018

Closing for now. Feel free to send this to #344, we can ship it there.

@mcollina mcollina closed this Jul 9, 2018
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.

3 participants