-
Notifications
You must be signed in to change notification settings - Fork 2
Add ETH estimates #28
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
all_params.append("async_req") | ||
all_params.append("_return_http_data_only") | ||
all_params.append("_preload_content") | ||
all_params.append("_request_timeout") | ||
all_params.append("mass_g") | ||
all_params.append("total_price_cents_usd") | ||
all_params.append("project_id") | ||
all_params.append("metadata") | ||
all_params.append("distance_m") | ||
all_params.append("transportation_method") | ||
all_params.append("package_mass_g") | ||
all_params.append("create_order") | ||
all_params.append("make") | ||
all_params.append("model") | ||
all_params.append("year") | ||
all_params.append("transaction_value_btc_sats") | ||
all_params.append("timestamp") | ||
all_params.append("gas_used") |
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.
seems like this list of params is defined for every request in this file and consist of all possible params rather than the ones needed just for this particular endpoint. Not a blocker but do you happen to know the reason for this?
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.
Yes, this is a limitation of the current template / code generation.
I'm adding a tracker here: https://app.asana.com/0/1200609773402056/1200634505135525/f
@@ -142,6 +142,18 @@ def test_create_bitcoin_estimate_transaction_value(self): | |||
estimate.data.mass_g, 200 | |||
) # not setting an exact value since this is changing daily | |||
|
|||
def test_create_ethereum_estimate_transaction_value(self): |
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.
Should there be tests for project_id
,timestamp
, and create_order
params as well?
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.
no blockers but we may want to beef up the tests to use the additional params
What
Why
SDK Release Checklist