search

ByzantineFailure / BPM-for-Discord

BPM for Discord's Desktop App. Includes one-click installers, update notifications, and custom script support.
GNU Affero General Public License v3.0
17 stars 8 forks source link

Allow Node versions higher than 4.2.X to be used to install (Issue #58) #89

Open 1byte2bytes opened 7 years ago

1byte2bytes commented 7 years ago

Since @ByzantineFailure asked for a PR, here it is! A bit of a breakdown of how this works.

$(echo "$@" | tr " " "\n" | sort -rV | head -n 1)

Let's have a look inside of this block, first.

  1. echo "$@" - First we print all the arguments of the version_ge function
  2. tr " " "\n" - We replace all spaces (which would be the separator between version numbers at the moment like "8.1.4 4.2.0") to newlines. Each version on it's own line.
  3. sort -rV - We use the sort command to sort by version numbers, in reverse, so the newer version is the top string.
  4. head -n 1 - Finally, this will output the top string.

test "[above block]" == "$1"; - This then checks if the output of the block above is equal to the first argument of the function, a.k.a. the version of Node already on the computer. If it's equal, the host Node is new enough! This will return true if it is.

NODE_VERSION=$(node --version | cut -c2-) - This is pretty simple, we get the Node version and cut off the first character, or the "v" in front of the numbers. Otherwise it messes things up.

if version_ge $NODE_VERSION "4.2.0"; then - This one should be pretty simple too. If the Node version is greater or equal to 4.2.0, then we've met the requirements and will use the native Node install.

Perhaps we should have a link to the PR explaining this as a comment in the script or something.

1byte2bytes commented 7 years ago

I tested the script against Node 4.2.0 and Node 0.10.2 and it does reject them and grab Node 4.2. So that works, even if it did pain me to run such old copies of Node on my system

Calinou commented 6 years ago

Recent Node.js versions (tested with 8.9.3 and 9.2.0) seem to work fine for installing the latest BPM version, but this pull request seems to have stalled. Should I send another pull request?

Besides, the downloaded Node.js version should be updated as 4.2.4 is very old by now.

1byte2bytes commented 6 years ago

I can resubmit the PR on an updated copy of BPM for Discord to get rid of the conflicts, but I still don't have a Windows machine to test on, nor do I know anything about PowerShell or much about batch. Can anyone help out with the Windows side at all?

ByzantineFailure commented 6 years ago

I should look into doing this, I suppose.

Can you resubmit a PR that pins it to the latest LTS version? I'll need to update the build process to do the same but it shouldn't be an enormous deal.

1byte2bytes commented 6 years ago

Yeah, I could. Can you wait until tonight or tomorrow in the afternoon? I'm having to catch up on a lot of work today.