Improve API consistency #85
Conversation
| @@ -229,3 +235,98 @@ impl Default for Eip712TransactionRequest { | |||
| } | |||
| } | |||
| } | |||
|
|
|||
| impl TryFrom<WithdrawRequest> for Eip712TransactionRequest { | |||
There was a problem hiding this comment.
Since a WithdrawRequest is a subset of Eip712TransactionRequests, shouldn't we implement a TryInto in that module? (following the principle that Eip712TransactionRequest shouldn't know about its children structures.
There was a problem hiding this comment.
The TryFrom function already implements the functionality of TryInto, but the reverse isn't necessarily true. By implementing TryFrom or From, you automatically gain the corresponding TryInto or Into trait respectively. This behavior is outlined in the Rust documentation on this trait.
| } | ||
| } | ||
|
|
||
| impl From<TransferRequest> for Eip712TransactionRequest { |
There was a problem hiding this comment.
Same as my comment for WithdrawRequests
| } | ||
| } | ||
|
|
||
| impl TryFrom<DeployRequest> for Eip712TransactionRequest { |
There was a problem hiding this comment.
Same as my comment for WithdrawRequests
| polling_time_in_seconds: Option<Duration>, | ||
| timeout_in_seconds: Option<Duration>, | ||
| ) -> Result<TransactionReceipt, ProviderError> { | ||
| let polling_time_in_seconds = polling_time_in_seconds.unwrap_or(Duration::from_secs(2)); | ||
| let mut timer = tokio::time::interval(polling_time_in_seconds); | ||
| let start = Instant::now(); | ||
|
|
||
| let transaction_receipt = | ||
| self.get_transaction_receipt(tx_hash) |
There was a problem hiding this comment.
Maybe we could use TryInto and replace the tx_hash with a generic parameter in order to avoid double calls to get_transaction_receipt in the scenario where we already have a TransactionReceipt at hand.
There was a problem hiding this comment.
I'm not sure that I understand the suggestion, you mean that the hash should implements the Try_into to a TransactionReceipt?
examples/simple_payment/main.rs
Outdated
There was a problem hiding this comment.
Are we testing examples in the CI?
There was a problem hiding this comment.
No, we are not at the moment
Codecov Report
@@ Coverage Diff @@
## main #85 +/- ##
==========================================
- Coverage 83.02% 73.13% -9.90%
==========================================
Files 13 16 +3
Lines 3170 2021 -1149
==========================================
- Hits 2632 1478 -1154
- Misses 538 543 +5
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
closes #83