Skip to content

Adding combined streams, parseFloats#39

Open
gemmell wants to merge 28 commits intoluzzif:masterfrom
gemmell:master
Open

Adding combined streams, parseFloats#39
gemmell wants to merge 28 commits intoluzzif:masterfrom
gemmell:master

Conversation

@gemmell
Copy link
Contributor

@gemmell gemmell commented Sep 1, 2019

This is all the changes I've made on my branch. In a nut shell,

  • added a bunch of parseFloats, probably didn't do all the places they are needed though
  • added the combined streams (you don't need a stream per symbol)

Take or leave whatever you want!

Copy link
Owner

@luzzif luzzif left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you so much for the contribution and sorry for the late response.

I'm going to add a linter to the project so that those spacing problems get automatically fixed once you align the PR fixed, then just the console log is a problem.



const priceAsString: string = price.toFixed(8);
console.log("BinanceApiClient says the price is" + priceAsString);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd remove this console.log

interval: CandlestickInterval,
onUpdate: ( update: CandlestickUpdate ) => any,
connectionTimeout?: number,
onLostConnection?: () => any ): Promise< void > {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd remove the spaces in the diamond notation here. And it's my bad, I should add a linter to the project.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants