-
Notifications
You must be signed in to change notification settings - Fork 170
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
refactor: split subscribe ticker and candle channels #610
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! Just left a few minor suggestions - let's merge this before #611
Co-authored-by: Adam Wozniak <29418299+adamewozniak@users.noreply.github.com>
…ork/umee into rafilx/subscribe-ticker-candle
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Just one minor nit
Co-authored-by: Aleksandr Bezobchuk <alexanderbez@users.noreply.github.com>
…ribe-ticker-candle
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM - what do you think about adding SubscribeCurrencyPairs
to the interface?
I thought about that, but what about Osmosis? who doesn't have websocket |
…ribe-ticker-candle
Could put in a no-op, not sure what best practice is to do here. Imo it might be easier for library users (peggo) to assume that the method will be implemented on all providers. Then again, making interfaces as least specific to implementations as possible also makes sense. |
I imagine we could add |
Shall we merge this PR or do we want to make further tweaks? |
imo we can merge, I can update the interface in #611 later today |
(cherry picked from commit d75ab49)
Description
websocket.CloseAbnormalClosure
to Kraken when EOF occurs to avoid having the followingAuthor Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!
to the type prefix if API or client breaking changeCHANGELOG.md
Reviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
I have...
!
in the type prefix if API or client breaking change