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

show only versions newer than NVM_MIN_VER if set #3277

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ryenus
Copy link
Contributor

@ryenus ryenus commented Jan 29, 2024

This is to fix #3250.

@ryenus
Copy link
Contributor Author

ryenus commented Jan 29, 2024

For example, running NVM_MIN_VER=18 nvm ls-remote --lts when v14 and v16 are installed:

image

@ryenus ryenus force-pushed the minver branch 2 times, most recently from a81e16a to b10554a Compare January 29, 2024 15:22
Copy link
Member

@ljharb ljharb left a comment

Choose a reason for hiding this comment

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

I like that it's done in awk :-) it'd be great to benchmark the difference to make sure this isn't slowing anything down meaningfully.

this will need tests, and also it'd be good to add a --min option to all the things that talk to the mirror (nvm ls-remote, nvm install, nvm version-remote) and then maybe nvm ls for good measure (altho that can be a follow-on if you like)

@ljharb ljharb added installing node Issues with installing node/io.js versions. feature requests I want a new feature in nvm! labels Jan 29, 2024
@ryenus
Copy link
Contributor Author

ryenus commented Jan 29, 2024

Performance-wise there's not really much change or overhead, you can trust that awk is fast :-) Sometimes it might even run faster, esp. when many older versions are skipped.

Meanwhile the other sibling functions and even the tests are something I'm not really familiar with, may you can chime in here? @ljharb

@ljharb
Copy link
Member

ljharb commented Jan 29, 2024

Sure - for the tests, there's existing tests for nvm ls-remote that employ mocks for the remote data, and a test that uses the env var, and asserts that it matches the mock minus the first N lines (since older versions aren't going to change, that number won't break over time), that would be sufficient. Similar approaches will work for the others.

For adding an option, if you grep for "ls-remote" you'll find the chunk of code where top-level commands are defined, and there's a bunch of existing examples that process the arguments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature requests I want a new feature in nvm! installing node Issues with installing node/io.js versions.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[ls-remote] only list active and/or maintained versions
2 participants