exchanges: Update exchange templates #1777

samuael · 2025-01-16T15:20:24Z

Selam(🕊️) be with you, team!

This PR includes minor updates to the exchange templates used for instantiating new exchanges.

Fixes # (issue)

Type of change

Please delete options that are not relevant and add an x in [] as the item is complete.

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
This change requires a documentation update

How has this been tested

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration and
also, consider improving test coverage whilst working on a certain feature or package.

go test ./... -race
golangci-lint run
Test X

Checklist

My code follows the style guidelines of this project
I have performed a self-review of my code
I have commented on my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation and regenerated documentation via the documentation tool
My changes generate no new warnings
I have added tests that prove my fix is effective or that my feature works
New and existing unit tests pass locally and on Github Actions/AppVeyor with my changes
Any dependent changes have been merged and published in downstream modules

… into update_tif

… into template_update

gbjk

Good progress!

Sorry; I'd like to remove the inconsistency in the exchange method receiver naming.
Some exchanges are naming it ex *Exchange and others have e *Exchange.
It seems to be following that the original exchange receiver was 2 characters, so ex is as well, but that doesn't make any sense.
If I'm not missing any other reason, can we make it consistently e everywhere?
Note: Specifically choosing e, and not ex in this request, to be idiomatic

Once that's done, I'll have another pass to do.

cmd/exchange_template/types.tmpl

cmd/exchange_template/wrapper.tmpl

exchanges/alphapoint/alphapoint.go

samuael · 2025-07-07T19:45:22Z

Good progress!

Sorry; I'd like to remove the inconsistency in the exchange method receiver naming. Some exchanges are naming it ex *Exchange and others have e *Exchange. It seems to be following that the original exchange receiver was 2 characters, so ex is as well, but that doesn't make any sense. If I'm not missing any other reason, can we make it consistently e everywhere? Note: Specifically choosing e, and not ex in this request, to be idiomatic

Once that's done, I'll have another pass to do.

Regarding the receivers naming, I am decision neutral(Pontius 😉 ) on this idea of replacing two variable receivers with "ex" and with just 'e' for single variable receiver for the rest. But, regarding consistency, I think, having the consistency in the specific exchange package is enough ... may be @thrasher- could give comment on this.

thrasher- · 2025-07-08T00:41:02Z

Good progress!
Sorry; I'd like to remove the inconsistency in the exchange method receiver naming. Some exchanges are naming it ex *Exchange and others have e *Exchange. It seems to be following that the original exchange receiver was 2 characters, so ex is as well, but that doesn't make any sense. If I'm not missing any other reason, can we make it consistently e everywhere? Note: Specifically choosing e, and not ex in this request, to be idiomatic
Once that's done, I'll have another pass to do.

Regarding the receivers naming, I am decision neutral(Pontius 😉 ) on this idea of replacing two variable receivers with "ex" and with just 'e' for single variable receiver for the rest. But, regarding consistency, I think, having the consistency in the specific exchange package is enough ... may be @thrasher- could give comment on this.

We should aim to be as consistent as possible, so in this case we can change them in the ./exchanges/ dir to be e. The original need for two letter chars was due to clashes with other template vars; e.g OKX 'so would clash with a var of o for order.X so the template tool would set OKX's reciever to ok

cranktakular

Got some changes I need.

In general, I'd also suggest making a template for rate limiting, and finishing the checklist at the start of this PR.

cmd/exchange_template/wrapper.tmpl

cmd/exchange_template/exchange_template.go

cranktakular · 2025-07-08T03:35:34Z

cmd/exchange_template/exchange_template.go

 	fmt.Printf("Output directory: %s\n", exchangeDirectory)

 	exch.CapitalName = cases.Title(language.English).String(exch.Name)
-	exch.Variable = exch.Name[0:2]


With this line being removed, do we still need checkExchangeName to have that length check? Smells like it was mostly to avoid panics here.

I think we should keep this check just because the naming should satisfy this check even-if it is not helpful for the receiver naming.

cmd/exchange_template/exchange_template_test.go

cmd/exchange_template/wrapper.tmpl

… into template_update

gbjk

Thanks.
Couple of new changes, and some outstanding existing changes.

cmd/exchange_template/websocket.tmpl

cmd/exchange_template/wrapper.tmpl

… into template_update

cranktakular

Got one little nit, but aside that, I can tACK.

cranktakular · 2025-07-15T01:22:21Z

cmd/exchange_template/exchange_template_test.go

@@ -65,18 +72,14 @@ func TestNewExchangeAndSaveConfig(t *testing.T) {
 			WS:   true,


Changing this to false adds a line of coverage for code you newly added, without losing any elsewhere.

If you want to keep this in case of issues, you can set up another exchange where this is false.

cranktakular

And that makes it a full-on tACK from me.

(At least for the exchange_template folder, I haven't reviewed the rest of the changes)

gbjk

Approved 👍

Minor nits only 🐒 and just a reminder in case you missed the suggestion that the newly private function doesn't need a comment 🦍

gbjk · 2025-07-15T19:12:14Z

cmd/exchange_template/websocket.tmpl


-// Subscriber sends a websocket message to receive data from the channel
-func (e *Exchange) Subscriber(_ subscription.List) error {
+// Subscribe sends a websocket message to receive data from the channel


🐒 Note: There was a change to the comment here. But in hindsight I'm not happy with that suggestion either. Just a minor update to plurarity.

Suggested change

// Subscribe sends a websocket message to receive data from the channel

// Subscribe sends websocket messages to receive data for a list of channels

gbjk · 2025-07-15T19:13:12Z

cmd/exchange_template/websocket.tmpl


-// Unsubscriber sends a websocket message to stop receiving data from the channel
-func (e *Exchange) Unsubscriber(_ subscription.List) error {
+// Unsubscribe removes a channel subscriptions from the websocket


🐒

Suggested change

// Unsubscribe removes a channel subscriptions from the websocket

// Unsubscribe sends websocket messages to stop receiving data for a list of channels

gbjk · 2025-07-15T19:14:38Z

cmd/exchange_template/websocket.tmpl


-// GenerateSubscriptions returns a list of subscriptions from the configured subscriptions feature
-func (e *Exchange) GenerateSubscriptions() (subscription.List, error) {
+// generateSubscriptions returns a list of subscriptions from the configured subscriptions feature


🦍 In case you missed it, I'm suggesting to remove this comment

Suggested change

// generateSubscriptions returns a list of subscriptions from the configured subscriptions feature

gbjk

All good, Thanks and Great work 👍

thrasher-

Last two changes and then this LGTG, thanks for addressing the feedback!

thrasher- · 2025-07-16T07:26:24Z

cmd/exchange_template/rest.tmpl

+	"github.com/thrasher-corp/gocryptotrader/exchanges/request"
+)
+
+// Exchange implements exchange.IBotExchange and contains additional specific api methods for interacting with {{.CapitalName}}


Suggested change

// Exchange implements exchange.IBotExchange and contains additional specific api methods for interacting with {{.CapitalName}}

// Exchange implements exchange.IBotExchange and contains additional specific API methods for interacting with {{.CapitalName}}

thrasher- · 2025-07-16T07:38:44Z

cmd/exchange_template/exchange_template_test.go

 			Name: testExchangeName,
 			REST: true,
-			WS:   true,
+			WS:   false,


Please expand this test to also include WS enabled as well, so we test both REST only and both enabled

thrasher-

Changes tACK, thanks!

samuael and others added 30 commits October 29, 2023 21:28

Added TimeInForce type and updated related files

5412e91

Linter issue fix and minor coinbasepro type update

0bbd6fb

Bitrex consts update

b786e75

added unit test and minor changes in bittrex

afa9695

Merge branch 'master' of http://github.com/thrasher-corp/gocryptotrader…

cb2619b

… into update_tif

Unit tests update

da4b3cb

Merge branch 'master' of http://github.com/thrasher-corp/gocryptotrader…

e41ab86

… into update_tif

Merge branch 'master' of http://github.com/thrasher-corp/gocryptotrader…

c9b88ab

… into update_tif

Fix minor linter issues

e11beef

Update TestStringToTimeInForce unit test

9add848

Merge branch 'master' of http://github.com/thrasher-corp/gocryptotrader…

aecc942

… into update_tif

Merge branch 'master' of http://github.com/thrasher-corp/gocryptotrader…

3a7ac71

… into update_tif

Merge branch 'master' of http://github.com/thrasher-corp/gocryptotrader…

44f1cbb

… into update_tif

Merge branch 'master' of http://github.com/thrasher-corp/gocryptotrader…

964ed95

… into update_tif

Merge branch 'master' of http://github.com/thrasher-corp/gocryptotrader…

05b4921

… into update_tif

Merge branch 'master' of http://github.com/thrasher-corp/gocryptotrader…

2179454

… into update_tif

Merge branch 'master' of http://github.com/thrasher-corp/gocryptotrader…

3043702

… into update_tif

Merge branch 'master' of http://github.com/thrasher-corp/gocryptotrader…

45b3596

… into update_tif

Merge branch 'master' of http://github.com/thrasher-corp/gocryptotrader…

318152b

… into update_tif

Merge branch 'master' of http://github.com/thrasher-corp/gocryptotrader…

cede5cb

… into update_tif

Merge branch 'master' of http://github.com/thrasher-corp/gocryptotrader…

3aa605a

… into update_tif

Merge branch 'master' of http://github.com/thrasher-corp/gocryptotrader…

6dd27b9

… into update_tif

Exchange test template change

c0ca501

A different approach

c09f439

Merge branch 'master' of http://github.com/thrasher-corp/gocryptotrader…

6b3e010

… into update_tif

fix conflict with gateio timeInForce

d82cc0c

Merge branch 'master' of http://github.com/thrasher-corp/gocryptotrader…

b453062

… into update_tif

minor exchange template update

b115b2b

Merge branch 'master' of http://github.com/thrasher-corp/gocryptotrader…

e64ae62

… into template_update

Minor fix to test_files template

917dd6b

thrasher- requested a review from gbjk July 7, 2025 02:22

gbjk requested changes Jul 7, 2025

View reviewed changes

update templates comments

fd07039

cranktakular requested changes Jul 8, 2025

View reviewed changes

samuael added 4 commits July 8, 2025 13:46

update templates and related comments

20d8897

renamed ex to e

eccce94

Merge branch 'master' of http://github.com/thrasher-corp/gocryptotrader…

8f6bdb2

… into template_update

Merge branch 'master' of http://github.com/thrasher-corp/gocryptotrader…

0f7df23

… into template_update

samuael requested review from cranktakular, gbjk and shazbert July 14, 2025 08:08

gbjk requested changes Jul 14, 2025

View reviewed changes

samuael added 2 commits July 14, 2025 22:26

Merge branch 'master' of http://github.com/thrasher-corp/gocryptotrader…

f0e0f07

… into template_update

update template comments

837b782

samuael requested a review from gbjk July 14, 2025 19:49

cranktakular approved these changes Jul 15, 2025

View reviewed changes

toggle WS to false to improve coverage

ae2f8f6

samuael requested a review from cranktakular July 15, 2025 05:57

cranktakular approved these changes Jul 15, 2025

View reviewed changes

gbjk approved these changes Jul 15, 2025

View reviewed changes

template comments update

da4bc56

gbjk approved these changes Jul 16, 2025

View reviewed changes

thrasher- requested changes Jul 16, 2025

View reviewed changes

samuael requested a review from thrasher- July 16, 2025 10:32

added test coverage to Ws enabled and minor changes

e67d9da

thrasher- approved these changes Jul 17, 2025

View reviewed changes

thrasher- merged commit 3f534a1 into thrasher-corp:master Jul 17, 2025
12 of 16 checks passed

thrasher- mentioned this pull request Jul 17, 2025

cmd/exchange_template, exchanges: Rid contributors from exchange readme #1978

Closed

		@@ -65,18 +72,14 @@ func TestNewExchangeAndSaveConfig(t *testing.T) {
		WS: true,

	// Subscribe sends a websocket message to receive data from the channel
	// Subscribe sends websocket messages to receive data for a list of channels

	// Unsubscribe removes a channel subscriptions from the websocket
	// Unsubscribe sends websocket messages to stop receiving data for a list of channels

	// Exchange implements exchange.IBotExchange and contains additional specific api methods for interacting with {{.CapitalName}}
	// Exchange implements exchange.IBotExchange and contains additional specific API methods for interacting with {{.CapitalName}}

exchanges: Update exchange templates #1777

exchanges: Update exchange templates #1777

Uh oh!

Conversation

samuael commented Jan 16, 2025

Type of change

How has this been tested

Checklist

Uh oh!

gbjk left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

samuael commented Jul 7, 2025

Uh oh!

thrasher- commented Jul 8, 2025

Uh oh!

cranktakular left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gbjk left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cranktakular left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cranktakular left a comment

Choose a reason for hiding this comment

Uh oh!

gbjk left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gbjk left a comment

Choose a reason for hiding this comment

Uh oh!

thrasher- left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thrasher- left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers