Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add get_depth #5

Merged
merged 1 commit into from
Mar 24, 2018
Merged

Conversation

rupurt
Copy link
Collaborator

@rupurt rupurt commented Mar 23, 2018

  • Adds passing test coverage
  • Records HTTP requests for later offline playback
  • Returns {:error, reason} tuples for error response payloads

/~https://github.com/binance-exchange/binance-official-api-docs/blob/master/rest-api.md#order-book

@dvcrn
Copy link
Owner

dvcrn commented Mar 23, 2018

Very cool, thanks a lot for the PR and going the extra step of adding tests!

Most of the other endpoints currently conform JSON data into structs (following the mentality explained here). Could you add a struct for this endpoint to keep things consistent? Something like ticker

@rupurt rupurt force-pushed the add-test-and-depth branch from f81e2d3 to 31a3b3c Compare March 23, 2018 16:51
@rupurt
Copy link
Collaborator Author

rupurt commented Mar 23, 2018

@dvcrn I force pushed the updates to this branch.

Sorry I didn't notice that pattern originally. I'm digging ExConstructor and the auto case conversions.

Cheers 🍻

- Adds passing test coverage
- Records HTTP requests for later offline playback
- Returns {:error, reason} tuples for error response payloads

/~https://github.com/binance-exchange/binance-official-api-docs/blob/master/rest-api.md#order-book
@rupurt rupurt force-pushed the add-test-and-depth branch from 31a3b3c to baa6e12 Compare March 23, 2018 16:54
@dvcrn
Copy link
Owner

dvcrn commented Mar 24, 2018

Thank you! ❤️

@dvcrn dvcrn merged commit d51ea7d into dvcrn:master Mar 24, 2018
@rupurt rupurt deleted the add-test-and-depth branch March 24, 2018 17:45
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