wbyoung / avn

Automatic Version Switching for Node
MIT License
1.14k stars 54 forks source link

appendScript to .bashrc for Ubuntu-based #50

Closed edsfocci closed 7 years ago

edsfocci commented 7 years ago

This change is due to Ubuntu's preference to use .bashrc for sourcing executables. It works better for me, and I don't need to log out to be able to use avn. I also re-worked the promises, which was necessary for my edit, but most of the original code's untouched, and should still work like before this change.

coveralls commented 7 years ago

Coverage Status

Coverage decreased (-6.2%) to 93.802% when pulling 820e4eecc836f29563fc60f583cfc579834488d8 on edsfocci:add-bashrc-to-setup into 24e1b1d2563064df5710669ed1ead75fb29b1dbf on wbyoung:master.

coveralls commented 7 years ago

Coverage Status

Coverage decreased (-6.2%) to 93.802% when pulling 820e4eecc836f29563fc60f583cfc579834488d8 on edsfocci:add-bashrc-to-setup into 24e1b1d2563064df5710669ed1ead75fb29b1dbf on wbyoung:master.

edsfocci commented 7 years ago

All the minor bugs are fixed. I don't know if your team has any philosophical or logical reasons to append the sourcing script to .bash_profile instead of .bashrc; feel free to enlighten me. I know that my Linux Mint 18 never came with .bash_profile in my home folder, and probably for a very good reason. Anyways, I was able to move the contents of the .bash_profile (1 line) that your module creates, to the bottom of my .bashrc and it works great. This fix is to automate that process for Ubuntu users only, to minimize conflicts with other operating systems.

wbyoung commented 7 years ago

Thanks for the work on this, @edsfocci.

I really do want this to be easier on everyone, but before this PR is merged, I want to ensure that it's being fixed for the long term. See also: https://github.com/wbyoung/avn/issues/31 where there's been some discussion including pointing out that getting this just right is actually quite difficult.

Here's what I'd want to know before accepting this PR:

After a quick glance at the code, the main thing I'd like to see different is to default to the Linux way which is probably more likely to be "standard" and have a special case for Mac OS X (darwin).

A whole can of worms gets opened as we start to consider both files, though, since one can source the other. Usually, if one is sourcing the other, we wouldn't want to edit that file. We'd want to edit the one that's not sourced.

I think the algorithm could be:

edsfocci commented 7 years ago

Ok, I made a pull request in a different branch, made according to your algorithm. I'm not sure if you want to do anything still with .zshrc (quick Google search shows that ArchLinux might use .zshrc). Also, in issue #49, the guy uses .profile in his Mac computer.

Working on your checklist will probably take some time. I can vouch for Linux Mint 18 (related to Ubuntu 16.04), and getting info on other Linux distros will probably take time (probably spend a few weekends). I won't be able to work with Mac OS X versions since I don't have a Mac unfortunately.

I'll probably work some more on this on the next couple of days.

wbyoung commented 7 years ago

@edsfocci awesome, I'll continue the conversation over on #51.