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

Return struct for insufficient balance error #17

Merged

Conversation

rupurt
Copy link
Collaborator

@rupurt rupurt commented Jun 28, 2018

No description provided.

@rupurt
Copy link
Collaborator Author

rupurt commented Jul 8, 2018

@dvcrn bump

@dvcrn dvcrn merged commit 40f0424 into dvcrn:master Jul 9, 2018
@dvcrn
Copy link
Owner

dvcrn commented Jul 9, 2018

I again didn't receive an email for this PR. Apologies for the delay and thanks for the contribution!

@rupurt
Copy link
Collaborator Author

rupurt commented Jul 9, 2018

No worries. Thanks for the merge!

Do you mind making a new hex package release?

@dvcrn
Copy link
Owner

dvcrn commented Jul 9, 2018

Published! https://github.com/dvcrn/binance.ex/releases/tag/v0.5.0

%{code: -2010, msg: "Account has insufficient balance for requested action."} = reason
}
}) do
{:error, %Binance.InsufficientBalanceError{reason: reason}}
Copy link
Owner

Choose a reason for hiding this comment

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

Actually looking at it now, It might be better to sub-package this to something like %Binance.Error.InsufficientBalance{} and use %Binance.Error{} for all other errors that are not implements as their own struct yet.

That and 'InsufficientBalance' already implies that the account doesn't have enough balance for the action so enforcing a :reason key is probably not needed too

But we can refactor that at a later point

@rupurt rupurt deleted the struct-insufficient-balance-error branch July 14, 2018 00:10
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.

None yet

2 participants