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

BTCMarkets: Add subscription configuration #1624

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

gbjk
Copy link
Collaborator

@gbjk gbjk commented Aug 20, 2024

BTCMarkets

  • Add subscription configuration
  • Add subscription.List.GroupByPairs and subscription.List.Authenticated
  • Group subscriptions by pairs to reduce subscription messages
  • Rename default config exchange name to match package name
  • Subscribe to heartbeat when not authenticated

Type of change

  • New feature (non-breaking change which adds functionality)

@gbjk gbjk added the review me This pull request is ready for review label Aug 20, 2024
@gbjk gbjk self-assigned this Aug 20, 2024
@gbjk gbjk force-pushed the feature/btcmarkets_sub_conf branch from 092733d to 8fd7801 Compare August 20, 2024 05:34
@gbjk gbjk force-pushed the feature/btcmarkets_sub_conf branch from 8fd7801 to c80b261 Compare August 20, 2024 06:12
Copy link

codecov bot commented Aug 20, 2024

Codecov Report

Attention: Patch coverage is 62.22222% with 17 lines in your changes missing coverage. Please review.

Project coverage is 38.27%. Comparing base (17c2ef2) to head (a759760).
Report is 2 commits behind head on master.

Files Patch % Lines
exchanges/btcmarkets/btcmarkets_websocket.go 32.00% 17 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1624      +/-   ##
==========================================
+ Coverage   36.37%   38.27%   +1.90%     
==========================================
  Files         422      422              
  Lines      183113   152325   -30788     
==========================================
- Hits        66602    58305    -8297     
+ Misses     108466    85976   -22490     
+ Partials     8045     8044       -1     
Files Coverage Δ
exchanges/btcmarkets/btcmarkets_wrapper.go 36.39% <100.00%> (+3.62%) ⬆️
exchanges/subscription/list.go 100.00% <100.00%> (ø)
exchanges/subscription/subscription.go 100.00% <ø> (ø)
internal/testing/exchange/exchange.go 46.90% <100.00%> (+4.04%) ⬆️
exchanges/btcmarkets/btcmarkets_websocket.go 47.30% <32.00%> (+7.45%) ⬆️

... and 394 files with indirect coverage changes

Copy link
Collaborator

@shazbert shazbert left a comment

Choose a reason for hiding this comment

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

Nice work! Just one nit.

exchanges/subscription/list.go Outdated Show resolved Hide resolved
@gbjk gbjk requested a review from shazbert August 28, 2024 07:17
Copy link
Collaborator

@shazbert shazbert left a comment

Choose a reason for hiding this comment

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

ch ACK. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: In review
Development

Successfully merging this pull request may close these issues.

3 participants