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

Change which to command -v #532

Merged
merged 1 commit into from Nov 5, 2018
Merged

Conversation

shadowspawn
Copy link
Collaborator

Pull Request Template:

Describe what you did

Fix some issues on an OS without which, such as Centos docker image.

How you did it

Changed which to command -v in the check_current_version routine.

Also added speech marks around variable dereference to fix error message that occurred before fixing above.

How to verify it doesn't effect the functionality of n

Checked n and n prune on MacOS and Centos.

If this solves an issue, please put issue in PR notes.

If this solves an issue, please include the output of issue that had problems and then the fixed output from the same command.

Before (with no which command):

# n prune
/usr/local/bin/n: line 263: which: command not found
/usr/local/bin/n: line 263: which: command not found
/usr/local/bin/n: line 625: [: node/10.12.0: unary operator expected

After:

# n prune

Squash any unnecessary commits to keep history clean as possible

Place description for the changelog in PR so we can tally all changes for any future release

Remove use of which and use generally available command -v.

@troy0820
Copy link
Collaborator

troy0820 commented Nov 5, 2018

This looks good. I'm going to try to pull it in today but trying to do them one at a time is interesting with the order I need them to work.

@troy0820 troy0820 merged commit 330e9ad into tj:master Nov 5, 2018
@shadowspawn shadowspawn deleted the issue/which_to_command branch December 18, 2018 05:53
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.

None yet

2 participants