Skip to content
This repository has been archived by the owner on Sep 25, 2020. It is now read-only.

Add requirements to README #288

Closed
wants to merge 3 commits into from
Closed

Conversation

dansimau
Copy link
Contributor

@dansimau dansimau commented Jun 17, 2016

Add a section to the README documenting the requirements for ringpop-node.

This should help people run into trouble when trying to npm install on a newer version of node1.

@motiejus
Copy link
Contributor

LGTM

@thanodnl
Copy link
Contributor

The change to the readme looks good and can be merged, but shall we also specify the version in the package.json under the engines section

@mennopruijssers
Copy link
Contributor

LGTM but let's add the engines-section as well :)

@thanodnl
Copy link
Contributor

thanodnl commented Jul 5, 2016

I'm not completely sure if 0.10.x is the right version. I was unable to do an install of the dependencies with the following versions:

  • 0.10.0
  • 0.10.1
  • 0.10.2
  • 0.10.3

All seem to have troubles with our package json dependencies section where the ^ is used to ask for compatible versions of packages. Versions that worked for me are the following:

  • 0.10.26
  • 0.10.32

Even though the problem is not with node but rather with nvm I wonder if we should specify a higher version of node to make sure that nvm install works. We can also figure out which version of nvm has support for ^ versions but I couldn't easily find when support was added. [email protected] has support (shipped with [email protected]) and [email protected] (shipped with [email protected]) does not have support.

@dansimau
Copy link
Contributor Author

dansimau commented Jul 5, 2016

Ah thanks @thanodnl for looking into this.

Should we specify ^0.10.32 then (that is the version we typically test with anyway)?

Maybe I should do a sanity check of versions above 0.10.32.

@dansimau
Copy link
Contributor Author

Travis tests run on v0.10.45 successfully so I'm just gonna merge this.

@mennopruijssers
Copy link
Contributor

LGTM!

@dansimau dansimau force-pushed the add-requirements-to-readme branch from dff7ce8 to 87a63a4 Compare July 13, 2016 09:39
@dansimau dansimau force-pushed the add-requirements-to-readme branch from 87a63a4 to eeca7d3 Compare July 13, 2016 09:50
@dansimau dansimau closed this in ee72383 Jul 13, 2016
@dansimau
Copy link
Contributor Author

Merged in commit ee72383.

@dansimau dansimau deleted the add-requirements-to-readme branch July 13, 2016 10:33
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants