Add missing fields to the Account model#295
Add missing fields to the Account model#295ankitsridhar16 wants to merge 3 commits intoalpacahq:masterfrom
Conversation
There was a problem hiding this comment.
Looks good to me. Thanks for doing this!
Some potential changes to consider, depending on @gnvk's preferences:
- change
BalanceAsoftoBalanceAsOf(capital "O") -- this reads better, but might be considered inconsistent with the JSON field name - keep the
Options*fields grouped together and thePending*fields
I have added the suggested changes to the PR @gcatlin can you review ? |
|
@ankitsridhar16 Looks good. Thanks! |
gnvk
changed the title
Account model
| PendingTransferIn string `json:"pending_transfer_in"` | ||
| PendingTransferOut string `json:"pending_transfer_out"` | ||
| IntradayAdjustments string `json:"intraday_adjustments"` | ||
| BalanceAsOf string `json:"balance_asof"` |
There was a problem hiding this comment.
This is a date, so I'd recommend to use civil.Date instead of a string.
gnvk
left a comment
gnvk
left a comment
There was a problem hiding this comment.
Please add / extend a unit test. If you do so, you will realise that the newly added fields are always empty, even when the server returns non-empty values for them. To fix that you'll need to regenerate the easyjson code with make generate.
I will check and ask if I have more doubts |
3 participants
resolves #288