major changes

[Jaraujo6]

It doesn't look like the owner of this npm module is around. perhaps you can publish your PR as a standalone npm package?

[askmike]

Hey! This PR is indeed very old.

I don't really see the added value and I think it's bad practice to allow users to store API credentials in flat file (json).

@Jaraujo6 what is in this PR that you like?

[Jaraujo6]

@askmike Thanks for responding!

If you're referring to the sample config file storing the API credentials, while I agree with the your general sentiment, it appears that the author of this PR account for that by gitignoring the config/dev and config/production files so that the credentials are never exposed on a github repo. It's no different that using environment variables in that regard, however I agree that there should be clear instructions stating that the developers need to create those files and not use the config sample provided (They state that passively on line 27-32 in example.js).

The real value of this PR however is the conversion from callbacks to promises/async functions. In my experience, callbacks are increasingly frustrating to work with if you have multiple transformations you need to perform on the response data. I find it much easier to use promises to manage the sequencing and processing of asynchronous data. I think that having this capability will make this library more attractive to other developers as well.

[13pass]

Thanks @Jaraujo6 you wrote it better than I would have. Anyway I suppose people dealing with crypto-currencies are (or should be) aware of software security basis. I understand that @askmike has some concerns about it. Anyway I'm not going to create a standalone npm module just for it. If you really want to use promises/async functions you can check https://github.com/ccxt/ccxt as an alternative as well. Ccxt is a multipurpose lib dealing with lots of different exchanges, Bitstamp is actually pretty well supported.

[askmike]

it appears that the author of this PR account for that by gitignoring the config/dev and config/production files so that the credentials are never exposed on a github repo. It's no different that using environment variables in that regard

Yes it is very different, gitignore files are only taken into account by git. Any other way of copying will simply copy this over. For example different version control systems, native OS copy, things like rsync, anything related to docker, backups, etc. I consider it a serious security vulnerability. The point of environment variables is that they are not on disk but in memory, not saying they are always safe. But at least they are not in plaintext on disk.

The real value of this PR however is the conversion from callbacks to promises/async functions. In my experience, callbacks are increasingly frustrating to work with if you have multiple transformations you need to perform on the response data. I find it much easier to use promises to manage the sequencing and processing of asynchronous data. I think that having this capability will make this library more attractive to other developers as well.

I do agree how handy promises are (note that you don't need async functions, since you can natively await any promise). However we don't really need to rewrite the whole lib in order to do that. This is an easier/cleaner way of turning the lib into promises if you want to:

const pify = require('pify');
const bitstamp = require('bitstamp');

const b = new Bitstamp();
pify(b.getTrades).then(() => console.log('voila')).catch(/* */)

That way all other people who don't want to use promises are not forced into using them. I created this lib for Gekko and Gekko specifically tries to work with NON promise based exchange APIs since unless error handling is 100% managed by the lib dealing with it is a pain since you constantly need to jump from catch to success and the other way around, for example see this: https://github.com/askmike/gekko/blob/gekko-trader/exchange/wrappers/coinfalcon.js#L52-L83

Anyway I suppose people dealing with crypto-currencies are (or should be) aware of software security basis.

It is my experience that this is a big problem, because to many people are not (I am exposed to a lot of developers through my project Gekko). I rather not publish libraries that might result in people getting robbed because they "should have known proper opsec" but didn't.