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

If jq is installed, use it to get values from package.json #810

Merged
merged 1 commit into from
Aug 16, 2024

Conversation

elibarzilay
Copy link
Contributor

Pull Request

jq is a popular choice for being installed, especially on CI machines, and using it is much faster than running node.

Problem

Using node can be problematic if it's not installed yet, and it also has a much higher overhead (= time) to start. Doesn't matter for an automated CI case, but in interactive use the potential doubling of running node is noticeable.

Solution

Use jq if it's installed. This is very straightforward use of jq, so prefer that over node if it is installed.

ChangeLog

  • Use jq to read values from package.json if it is installed.

Closes #807.

`jq` is a popular choice for being installed, especially on CI machines,
and using it is much faster than running node.
if command -v jq &> /dev/null; then
command -v jq && jq --version
else
echo "jq not found, can be used to get package.json values faster than node"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
echo "jq not found, can be used to get package.json values faster than node"
echo "jq not found. Optional install used for reading engine version from package.json."

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I don't mind the phrasing -- I forgot to say that you should make it whatever fits...

Copy link
Collaborator

@shadowspawn shadowspawn left a comment

Choose a reason for hiding this comment

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

I think the combination of a speed boost and allowing use of n engine without node installed make this worthwhile.

LGTM. One wording suggestion.

@shadowspawn shadowspawn changed the base branch from master to develop August 16, 2024 04:12
@shadowspawn shadowspawn added the pending release Merged into a branch for a future release, but not released yet label Aug 16, 2024
@shadowspawn shadowspawn merged commit b496b82 into tj:develop Aug 16, 2024
@shadowspawn shadowspawn removed the pending release Merged into a branch for a future release, but not released yet label Sep 6, 2024
@MikeMcC399
Copy link

@shadowspawn

Thanks for this great package!

Please label 10.0.0 as latest release so it shows up as such on https://github.com/tj/n/releases. Currently 9.2.0 holds the latest label.

@shadowspawn
Copy link
Collaborator

Labelled as latest now on GitHub releases page. Thanks.

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.

Finding engine is pretty slow
3 participants