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

normalise versions for target == this comparison #716

Closed
wants to merge 1 commit into from
Closed

normalise versions for target == this comparison #716

wants to merge 1 commit into from

Conversation

rvagg
Copy link
Member

@rvagg rvagg commented Sep 8, 2015

For the node-gyp rebuild case, everything is calculated fine for configure but then when it gets to install it's passing in an argv[0] with the version number and the v is stripped from that version but we are comparing with process.version which has a v so the naive comparison says it's not the same version and therefore we are not targetting the "default".

The solution here is to simply normalise process.version and match it to the already normalised target version so the comparison is fair.

Reported here for 4.0.0-rc.4

@rvagg rvagg mentioned this pull request Sep 8, 2015
10 tasks
@@ -28,6 +28,9 @@ function processRelease (argv, gyp, defaultVersion, defaultRelease) {
// flatten version into String
version = versionSemver.version

// defaultVersion should come from process.version so ought to be valid semve
Copy link
Member

Choose a reason for hiding this comment

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

typo: semver

@rvagg
Copy link
Member Author

rvagg commented Sep 8, 2015

thanks @targos, fixed

rvagg added a commit that referenced this pull request Sep 8, 2015
@rvagg
Copy link
Member Author

rvagg commented Sep 8, 2015

landing to get this into 4.0.0-rc.5

@rvagg rvagg closed this Sep 8, 2015
@rvagg rvagg deleted the normalise-version-comparison branch September 8, 2015 11:01
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.

2 participants