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

Update nodejs package attr to fit https://github.com/NixOS/nixpkgs/pull/199835 #310

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

SuperSandro2000
Copy link

This is a none backwards compatible change that is required after NixOS/nixpkgs#199835

Copy link

@TSRBerry TSRBerry left a comment

Choose a reason for hiding this comment

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

Lgtm, just think the default version should be updated now.

@@ -57,7 +57,7 @@ var supplementNix = "supplement.nix";
var nodeEnvNix = "node-env.nix";
var lockJSON;
var registries = [];
var nodePackage = "nodejs-14_x";
var nodePackage = "nodejs_14";

Choose a reason for hiding this comment

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

Suggested change
var nodePackage = "nodejs_14";
var nodePackage = "nodejs_18";

Current node LTS version is v18, I'd prefer to use that as the default.

(I realize it might not have been the current LTS version at the time this PR was created.)

@@ -2,7 +2,7 @@

{pkgs ? import <nixpkgs> {
inherit system;
}, system ? builtins.currentSystem, nodejs ? pkgs."nodejs-14_x"}:
}, system ? builtins.currentSystem, nodejs ? pkgs."nodejs_14"}:

Choose a reason for hiding this comment

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

Suggested change
}, system ? builtins.currentSystem, nodejs ? pkgs."nodejs_14"}:
}, system ? builtins.currentSystem, nodejs ? pkgs."nodejs_18"}:

Same reason as above.

@SuperSandro2000
Copy link
Author

The point of this PR was not to bump the LTS version but to resolve the usage of aliases created through the rename of the node packages. I am sure bumping the node LTS versions has further impact which I currently lack the motivation to deal with.

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