Skip to content
This repository has been archived by the owner on Dec 6, 2023. It is now read-only.

Abort if current and target Node versions are the same (closes #78) #87

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

Conversation

alexilyaev
Copy link

@alexilyaev alexilyaev commented Aug 27, 2019

I'm trying to handle #78.

The implementation is very simple and probably doesn't cover edge cases.
But it works for me.
I'd be glad to expand upon this, but first wanted to get some traction going.

Also, some tests are failing, not sure how to fix that.
Maybe move the solution to the Node.js side.
I did in Bash thinking it would be much faster.

@alexilyaev
Copy link
Author

@wbyoung Hope you're still active here.

@wbyoung
Copy link
Owner

wbyoung commented Aug 28, 2019

I understand the point of wanting to optimize this, but I'm reluctant to make this change for a few reasons:

  1. The process of activating a version of node could change something besides just simply the version number (I thought I left this comment somewhere before, like on When switching directories, first compare current and target Node version before proceeding, and abort AVN activation if they are the same #78, but guess I forgot when that was first opened). For instance, it could change things about which global npm modules are available (similar to RVM gemsets).
  2. The file may not be just plain text and may not stay that way in the future (see Read node version from package.json #10).
  3. This doesn't seem to be the right solution to solving a performance problem. The performance problem should be addressed rather than adding completely to this system.
  4. Shell support & fragility is being increased by adding this complexity. One motivation of the current design is to keep as much as possible written in Node to avoid shell specific issues.

If we can resolve these concerns, then I think this is a go. But I'm skeptical at this point.

Also, I honestly don't use this project any more. I've tried to find time to finalize converting it to Babel & more modern JS, but haven't even completed that. So if you're interested in contributing, someone helping tackle and maintain complexity could help push things like this forward.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants