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

cmd/exchange_template: Update docs #1494

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions cmd/exchange_template/test_file.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ func TestMain(m *testing.M) {
{{ if .WS }} exchCfg.API.AuthenticatedWebsocketSupport = true {{ end }}
exchCfg.API.Credentials.Key = apiKey
exchCfg.API.Credentials.Secret = apiSecret
exchCfg.Enabled = true
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there anything else that should be changed? I only ask because there is no other TestMain in /exchanges/ that has this present and they work with authenticated endpoints. If this is an issue for them, could you update all of those?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Never mind! Vague error messages led me here, but the fundamental issue was no assets/pairs being set in configtest.json on initial generation, which is the more graceful fix in general.

Maybe we could leave a comment about that somewhere in this tool, or in the files generated by it?

Copy link
Collaborator

Choose a reason for hiding this comment

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

All G! Yeah that's fine. Adding some details to suggest that you should populate configtest.json with asset/pairs before running tests would be good. Whether it's in ADD_NEW_EXCHANGE.md, commentary in the tool or otherwise I'm happy with. We're lacking in documentation, so any extra details is appreciated

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've now done so in the printed output from running the tool. Please let me know what you think of the wording there.


err = {{.Variable}}.Setup(exchCfg)
if err != nil {
Expand Down
Loading