Various Modifications #2

Merged
merged 7 commits into from Sep 5, 2013

Conversation

Projects
None yet
2 participants
Contributor

johnwarden commented Aug 13, 2013

Hey,

So this package was a great starting point for me working on a mtgox daytrade app. I ended up making a bunch of modifications to it as I encountered various challenges.

The modifications include most importantly some reconnection logic, because I found mtgox was constantly disconnecting mysteriously, and for some reason the socket.io library wasn't able to reconnect and just ended up hanging. This code will detect that scenario and automatically kill and recreate the client.

I have also added the ability for the user to optionally choose which channels to subscribe to, as well as the optional ability to subscribe to client-specific channels (e.g. my individual trades) if the user includes a keys.js file.

Also made misc changes that made this module more useable in the Npm/Meteor environment I was using.

I'm not sure that all these changes make sense to you, especially some backwords-compatibility-breaking changes. But take a look and tell me what you think!

@olalonde olalonde and 1 other commented on an outdated diff Aug 22, 2013

var getChannel = function(key) {
return MTGOX_CHANNELS.filter(function(channel) {
- return (channel.key == key || channel.private == key);
+ return (channel.key == key || channel.private == key || channel.key == key);
@olalonde

olalonde Aug 22, 2013

Owner

Why is there 2 channel.key == key? Could you remove this please!

@johnwarden

johnwarden Aug 23, 2013

Contributor

Will do.

Owner

olalonde commented Aug 22, 2013

Thanks for this! I haven't tested it because I don't have time but I will trust you did ;) Please just make the edit I suggested before I merge.

johnwarden added some commits Aug 23, 2013

@johnwarden johnwarden Fixed issue mentioned by olalonde. Factored console formatting out to…
… separate file. Little cleanup on console output. Put back 'connect' method for better backwards compatibility.
7d0ef8f
@johnwarden johnwarden Fixed issue mentioned by olalonde. Factored console formatting out to…
… separate file. Little cleanup on console output. Put back 'connect' method for better backwards compatibility.
929b974
Owner

johnwarden commented on 929b974 Aug 23, 2013

Okay I've made that one change, and did a little more refactoring and reformatting.

demo.js still runs just as it used to, except for the logic to destroy and recreate the socket if its idle to long.

Note there is still one backwards compatible change: all the events pass on just the relevant part of the message, not the full message (e.g. 'trade' just passes on the trade, 'ticker' just the ticket, etc.). So if you upgrade you'd need to change your code just a bit.

@olalonde olalonde added a commit that referenced this pull request Sep 5, 2013

@olalonde olalonde Merge pull request #2 from johnwarden/master
Various Modifications
4bfa569

@olalonde olalonde merged commit 4bfa569 into olalonde:master Sep 5, 2013

Owner

olalonde commented Sep 5, 2013

Ok, I published under 0.1.0 in npm.

Contributor

johnwarden commented Sep 6, 2013

cool!

On Sep 5, 2013, at 2:47 PM, Olivier Lalonde notifications@github.com wrote:

Ok, I published under 0.1.0 in npm.


Reply to this email directly or view it on GitHub.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment