Skip to content

Conversation

@mstijak
Copy link
Contributor

@mstijak mstijak commented Mar 1, 2018

PR for #47

@coveralls
Copy link

Coverage Status

Coverage decreased (-23.7%) to 69.565% when pulling 9bca494 on mstijak:typings-1 into d91ab20 on HyperCubeProject:master.

@balthazar balthazar added the types label Mar 1, 2018
@balthazar
Copy link
Collaborator

@bennyn Looks good to you?

isBuyerMaker: boolean;
}

interface MyTrade {
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The name Trade is already used for websocket trade events.

index.d.ts Outdated
price: string;
origQty: string;
executedQty: string;
status: string;
Copy link
Contributor

Choose a reason for hiding this comment

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

status: string -> status: OrderStatus

isBestMatch: boolean;
}

interface QueryOrderResult {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can interface QueryOrderResult and interface Order be merged?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No. Order uses transactTime instead of time and provides some additional fields.

isWorking: boolean;
}

interface CancelOrderResult {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is that a Result? To me it looks like a Request because it is created when calling cancelOrder.

The Java API also names it CancelOrderRequest:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it is a Result.

@bennycode
Copy link
Contributor

Thanks for clarification, @mstijak. Approved!

@balthazar balthazar merged commit d2ebbc5 into ccxt:master Mar 4, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants