Skip to content
This repository was archived by the owner on Jan 10, 2023. It is now read-only.

Ox60#9

Open
dorbor wants to merge 3 commits into
devfrom
ox60
Open

Ox60#9
dorbor wants to merge 3 commits into
devfrom
ox60

Conversation

@dorbor
Copy link
Copy Markdown
Contributor

@dorbor dorbor commented Dec 6, 2022

No description provided.

Copy link
Copy Markdown
Contributor

@amin-nejad amin-nejad left a comment

Choose a reason for hiding this comment

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

Just a small change requested. Another thing is that the ticket number is wrong - it doesn't matter now but please ensure this is correct going forward

Comment on lines +29 to +31
body.weWant.issuer === "bitstamp"
? BitStampIssuers[body.weWant.currency]
: GateHubIssuers[body.weWant.currency];
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't think we should be defaulting to "gatehub" if the provided issuer is not "bitstamp". Anything that can lead to unexpected behaviour should be avoided I think. We should just enforce that the issuer needs to be provided so they have to explicitly provide either "gatehub" or "bitstamp"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This also means the python script needs to be updated to add the issuer to the API call that is made

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants