add method WsCombinedKlineServe to Binance Future
#258
Conversation
Codecov ReportPatch coverage has no change and project coverage change:
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more Additional details and impacted files@@ Coverage Diff @@
## main #258 +/- ##
==========================================
- Coverage 40.44% 40.06% -0.38%
==========================================
Files 19 19
Lines 2893 2920 +27
==========================================
Hits 1170 1170
- Misses 1647 1674 +27
Partials 76 76
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Hi @ramilexe, nice job!
I would like to understand what is the benefit of including this new signature in the exchange interface.
| @@ -14,6 +15,8 @@ import ( | |||
| "github.com/rodrigo-brito/ninjabot/tools/log" | |||
| ) | |||
|
|
|||
| var _ service.Feeder = &Binance{} | |||
There was a problem hiding this comment.
What is the purpose of this instance? To force the implementation of the interface?
| } | ||
| } | ||
| }() | ||
| go b.candleSubscription(ctx, ha, ccandle, cerr, pair, period, nil, false) |
There was a problem hiding this comment.
why pairs parameter is nil here?
There was a problem hiding this comment.
Because method candleSubscription handles both cases: for single pair-period (CandlesSubscription) and for multiple pairs (CandlesSubscriptionCombined)
| @@ -516,6 +519,10 @@ func (b *Binance) CandlesSubscription(ctx context.Context, pair, period string) | |||
| return ccandle, cerr | |||
| } | |||
|
|
|||
| func (b *Binance) CandlesSubscriptionCombined(_ context.Context, _ map[string]string) (chan model.Candle, chan error) { | |||
| panic("not implemented") | |||
There was a problem hiding this comment.
We do not have this for Spot market?
There was a problem hiding this comment.
I suggest implementing it in following PR to not overcomplicate this one
Do you have idea how we can do it without including new function into interface? |
If your idea is just to optimize the Are you consuming the |
Related to #257