Closed andreineculau closed 4 years ago
Neat! As you can see we still fetches node through brew since it has not yet caused any build problems on our build servers. If we start using nvm, then perhaps this script will no longer be necessary?
If we start using nvm, should we replace this script wth install-node.inc.sh which would use just nvm or brew or apt or our own bash install, depending on the platform/FORCE_BREW setting?
@weetmuts
FWIW this is off-topic, because I can't see how I should make any changes to this PR, but I'm game :)
As I wrote in the #182 description, nvm is quite dead and intricate even if popular, so I wouldn't choose nvm over brew as the default for managing node. And #182 is not a bedrock for "we start using nvm" (everywhere). It merely makes support-firecloud to integrate with nvm, if one really needs a specific node version, either as a technically-driven or process-driven requirement (i.e. if one is in a AAA-enterprise and has strict check rules for bumping a dependency).
Worth mentioning that I'm comparing nvm and brew while fully aware that nvm gives sandboxing functionality which brew doesn't i.e. each project can choose to run a different node version. But that need for sandboxing should be the exception, not the default.
which would use just nvm or brew or apt or our own bash install, depending on the platform/FORCE_BREW setting
see https://github.com/tobiipro/support-firecloud/pull/182#issuecomment-685588739
AFAICS the problem 6 months ago was a need for increased stability and speed. What I intend to do in my fork is
This fix is perfectly fine. :-) I was just thinking about how to install node in the future.
But the macosx build hangs. Do you know why?
AFAIK it's an intermitent Travis CI bug, not specific to macos, but definitely more common. Restarted the build to confirm.
While porting nvm functionality in #182 I also bumped into other "small" fixes, which might be good to port, especially the "don't change working directory" fix.