-
Notifications
You must be signed in to change notification settings - Fork 15
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
dist-indexer: fix libuv version generation #6
Conversation
Beginning in nodejs/node@537a4ba, `deps/uv/include/uv-version.h` moved to `deps/uv/include/uv/version.h`, so add that as a possible path where libuv's version might be stored. Refs: nodejs/node#21466
Once this has been approved, a release has been published to npm, and the new package has been installed on the web server, the following command should be run through the
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rubber-stamp LGTM
dist-indexer.js
Outdated
} | ||
}) | ||
|
||
fetch(uvVersionUrl[3], gitref, function (err, rawData) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you're going to need to nest this inside the previous fetch()
otherwise it's going to run in parallel and likely conflict. i.e. indent all of this plus the two lines below it by 2 spaces and move the })
just above here down to the bottom
Sorry about that, fixed the indentation. |
done and deployed and appears to be working nicely, changes may take a bit of time to show up in /release/ and a bit longer in the other directories, if anyone's impatient for forced updates I can give it a kick |
Beginning in nodejs/node@537a4ba,
deps/uv/include/uv-version.h
moved todeps/uv/include/uv/version.h
, so addthat as a possible path where libuv's version might be stored.
Refs: nodejs/node#21466
Unrelated note, but cc @Trott in case there are folks looking to get involved somewhere in Node, this is a low visibility with super high impact codebase (in JS) that could use some good refactoring. Happy to help mentor/review PRs if there are any takers...