Skip to content
This repository has been archived by the owner on Jul 24, 2024. It is now read-only.

Update module version for Node 8 #1969

Merged
merged 2 commits into from
May 16, 2017
Merged

Update module version for Node 8 #1969

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
348a3b1
Update module version for Node 8
xzyfer May 16, 2017
f783a9c
Premptively account for module version 57
xzyfer May 16, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion lib/extensions.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,8 @@ function getHumanNodeVersion(abi) {
case 47: return 'Node.js 5.x';
case 48: return 'Node.js 6.x';
case 51: return 'Node.js 7.x';
case 54: return 'Node.js 8.x';
case 55: return 'Node.js 8.x';
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor, but we chould just fall through here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

True. I did them as discrete commits so I didn't think about it. I want to update this handling so that it only shows as a helpful warning rather than erring out.

Copy link

Choose a reason for hiding this comment

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

can't we use a dictionary lookup instead of a switch?

function getHumanNodeVersion(abi) {
	var humanNodeVersions={
	   11:'Node 0.10.x',
     14:'Node 0.12.x',
     42:'io.js 1.x',
     43:'io.js 1.1.x',
     44:'io.js 2.x',
     45:'io.js 3.x',
     46:'Node.js 4.x',
     47:'Node.js 5.x',
     48:'Node.js 6.x',
     51:'Node.js 7.x',
     55:'Node.js 8.x',
     57:'Node.js 8.x'
	}
	var version = parseInt(abi || process.versions.modules, 10);
	return humanNodeVersions[version]||false;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We could, but I prefer the switch.

case 57: return 'Node.js 8.x';
default: return false;
}
}
Expand Down