search

Learn-by-doing / bitstarter

A crowd funding website that uses bitcoin
MIT License
23 stars 28 forks source link

Support for windows #41 #64

Closed bokova closed 7 years ago

bokova commented 7 years ago

Addresses https://github.com/Learn-by-doing/bitstarter/pull/41 - use bcrypt-nodejs on Windows

What I did:

  1. I modified the package.json, making bcrypt an optional dep. Thus the install won't fail. On Windows you'll get bcrypt-nodejs only. On Unix you'll get both bcrypt and bcrypt-nodejs installed. I got the inspiration from http://stackoverflow.com/a/26069595 See 'danger' note there.

  2. I created platform-dep.js where the bcrypt v bcrypt-nodejs logic resides. If bcrypt is present, it'll use that. If not, it'll use bcrypt-nodejs. I did it in a way in which you can add furhter exports later.

  3. I replaced references ('require') to bcrypt with platform-dep.js

  4. Apparently, when using bcrypt-nodejs the bcrypt.hash function in create-user.js requires a third callback progress. I added null to the code. I hope this works for Unix too. Can't test. See a bit of background here http://stackoverflow.com/a/21116806

  5. Sorry for the mess with extra commits. Charlie's gonna decline the PR anyway so it doesn't matter. :P

bokova commented 7 years ago

Apart from fixing that lint issue we'll have to address 4. as well. Null as the third parameter breaks the script on OSX

bokova commented 7 years ago

@chill117 please merge this. Not sure why it doesn't show Travis CI status, but it passed on my account

chill117 commented 7 years ago

Cannot merge because of conflicts with upstream/master.

You will have to do a rebase like this:

git rebase upstream/master

It will show some files with conflicts that you will have to manually resolve (by editing the conflicted files) and then finish the rebase like this:

git rebase --continue