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

Update versions #565

Merged
merged 2 commits into from
Mar 11, 2016
Merged

Update versions #565

merged 2 commits into from
Mar 11, 2016

Conversation

MylesBorins
Copy link
Contributor

  • path resolving now platform agnostic
  • update versions
    • done using scripts/load-versions

@ghost
Copy link

ghost commented Mar 9, 2016

Travis build passed 👍

@williamkapke
Copy link
Contributor

I was curious about this code- I can't tell how it is used now. There were uses in the code of require('./versions.json') but they have been removed. Now there just seems to be a link to the makefile... but I don't understand how that is used. If you're in this file- do you mind adding a comment indicating why/where version.json is used? (assuming you know) I would be super helpful for the next person.

@MylesBorins
Copy link
Contributor Author

@williamkapke I have no idea... I'll do a code audit later today and try and find out what's going on

@phillipj
Copy link
Member

phillipj commented Mar 9, 2016

In nodejs.org's early days, versions.json was used to read which versions of io.js and Node.js was available. Today we're using the node-version-data module to retrieve this data from https://{nodejs|iojs}.org/download/release/index.json. In other words, we don't use versions.json anymore IIRC.

@williamkapke
Copy link
Contributor

@phillipj yes, I churned through the commits to discover that- however when @rvagg switched it to his module, there was an if statement put around the part that writes it to disk when it is run as the target of the node process only. Making me think there was a real use for when it is called from make. Also, his commit message was load versions on build

@phillipj
Copy link
Member

phillipj commented Mar 9, 2016

I've been wanting to make loading of versions offline compatible, meaning we could decide to actually use versions.json at startup for convenience sake with a special CLI parameter. When commuting I've noticed the pain of being required to be online.

That's the only real use of versions.json I know of today.

@phillipj
Copy link
Member

Merging this as whether or not too keep versions.json should be discussed in a separate issue.

phillipj added a commit that referenced this pull request Mar 11, 2016
@phillipj phillipj merged commit 3c21e21 into master Mar 11, 2016
@phillipj phillipj deleted the update-versions branch March 11, 2016 20:03
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