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

Include the Node.js and npm version in the star.json manifest. #8956

Merged
merged 2 commits into from
Aug 2, 2017

Conversation

abernix
Copy link
Contributor

@abernix abernix commented Jul 28, 2017

This makes it possible to know exactly which version of Node.js and npm were used by the meteor tool which the bundle was built from by making new values available alongside the existing meteorRelease value in star.json: nodeVersion and npmVersion. The version of the star.json has remained the same since this is only adding new properties, not removing any existing. This preserves the existing .node_version.txt for compatibility reasons, but it's reasonable to believe that may be deprecated in a future version.

Those using .node_version.txt should consider using the nodeVersion value in star.json, though it should be noted that it does not include the v(ersion) prefix as the .node_version.txt file did.

This makes it possible to know exactly which version of Node.js and npm
were used by the `meteor` command from which the bundle was built from.
@abernix
Copy link
Contributor Author

abernix commented Jul 28, 2017

I'll add a History.md entry, after this passes CI.

@abernix abernix requested a review from benjamn July 28, 2017 11:44
@abernix abernix added this to the Release 1.5.2 milestone Aug 1, 2017
benjamn
benjamn previously requested changes Aug 2, 2017
@@ -14,6 +14,7 @@ var buildmessage = require('../utils/buildmessage.js');
var utils = require('../utils/utils.js');
var runLog = require('../runners/run-log.js');
var Profile = require('../tool-env/profile.js').Profile;
import { version as npmVersion } from 'npm';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be export { version as npmVersion } from "npm", and then you don't need to export it below!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do prefer this syntax! But opted for this technique to make the meteorNpm API feel more whole (as in, it seemed like the meteorNpm api should have a npmVersion exposed).

@benjamn benjamn dismissed their stale review August 2, 2017 15:49

Not super important, on second thought.

@benjamn benjamn merged commit 4665f2a into devel Aug 2, 2017
@abernix abernix deleted the abernix/add-node-and-npm-to-star.json branch August 2, 2017 16:35
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.

None yet

2 participants