-
-
Notifications
You must be signed in to change notification settings - Fork 345
REFACTOR: [types] Add UUID to OrderQuery #2047
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
Conversation
pkg/exchange/coinbase/exchange.go
Outdated
cbOrder, err := req.Do(ctx) | ||
if err != nil { | ||
return nil, errors.Wrapf(err, "failed to get order: %v", q.OrderID) | ||
return nil, errors.Wrapf(err, "failed to get order: %v", q.UUID) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In line 383-387, we may query by q.OrderID. Should we add q.OrderID into the error message ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The order id is a hash value of q.UUID
.
I was thinking knowing q.UUID
is sufficient to know q.OrderID
.
But I think you get a point.
If I choose to support q.OrderID
as query condition as well, I should include it into the message as well.
I will include the entire q
into the message with %+v
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done.
pkg/types/order.go
Outdated
@@ -260,6 +260,7 @@ type OrderQuery struct { | |||
Symbol string | |||
OrderID string | |||
ClientOrderID string | |||
UUID string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
call it OrderUUID? Align with the OrderID field
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
cdb6133
to
885bf04
Compare
885bf04
to
322a994
Compare
No description provided.