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.

Sign up for GitHub

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

Jump to bottom

jenkins: refactor to allow external node version #1265

Merged
merged 3 commits into from
Oct 2, 2018
Merged

jenkins: refactor to allow external node version #1265

merged 3 commits into from
Oct 2, 2018

Conversation

mhdawson
Copy link
Member

Allow NODE_MAJOR_VERSION to be set externally

joaocgreis

This comment was marked as off-topic.

Sign in to view

@mhdawson
Copy link
Member Author

@joaocgreis thanks !

maclover7

This comment was marked as off-topic.

Sign in to view

refack

This comment was marked as off-topic.

Sign in to view

@mhdawson
jenkins: refactor to allow external node version
9065d8f 
Allow NODE_MAJOR_VERSION to be set externally
@mhdawson
Copy link
Member Author

mhdawson commented Aug 2, 2018

Seeing failures in test release job with this branch, going to rebase to see if its missing changes.

refack

This comment was marked as off-topic.

Sign in to view

@mhdawson mhdawson mentioned this pull request Aug 20, 2018
Closed
2 tasks
richardlau

This comment was marked as off-topic.

Sign in to view

@richardlau richardlau mentioned this pull request Sep 27, 2018
Closed
@mhdawson
Copy link
Member Author

Will look and try to land this On Tuesday oct 2.

@refack
Copy link
Contributor

refack commented Sep 28, 2018

Will look and try to land this On Tuesday oct 2.

If you could check if it makes sense to use NODEJS_MAJOR_VERSION (with the JS), since it's rather widely used.

@mhdawson
Copy link
Member Author

mhdawson commented Oct 1, 2018

@refack will do.

@mhdawson
Copy link
Member Author

mhdawson commented Oct 2, 2018

Ok updated to use NODEJS_MAJOR_VERSION and validated in N-API testing and release job testing that it looks ok. Going to land.

@mhdawson mhdawson merged commit f48fc38 into nodejs:master Oct 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@richardlau richardlau richardlau left review comments

@refack refack refack approved these changes

@joaocgreis joaocgreis joaocgreis approved these changes

@maclover7 maclover7 maclover7 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@mhdawson @refack @joaocgreis @maclover7 @richardlau