-
Notifications
You must be signed in to change notification settings - Fork 3
Replace timeout function with external dependency #179
Conversation
Motivated by the combination of wanting to test `tryExecuteSirenAction` and realising that we would just be testing `timeoutPromise`. We have already decided to replace exactly the same function in `comit-rs` (comit-network/comit-rs#2330 (comment)), so replacing it here has the nice consequence of not needing to write a new test, since it's not our own code. Side-effects: - Move `TryParams` to `swap.ts` since it's directly related and only used for executing a swap. - Rename `util/timeout_promise.ts` to `util/sleep.ts`, since it now only holds a `sleep` function.
Are you sure the changelog does not need updating? |
2 similar comments
Are you sure the changelog does not need updating? |
Are you sure the changelog does not need updating? |
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.
Nice work!
Correct me if I'm wrong: Globally the TryParams
are exported the same way?
Because we do use them in the example. If the import in the examples has to be adapted this would be a breaking change.
Good point. I think it's exported the same, but I will quickly check locally. |
|
bors r+ |
Build succeeded |
Fixes #120 (imo).
Straight from the commit
Motivated by the combination of wanting to test
tryExecuteSirenAction
and realising that we would just be testingtimeoutPromise
. We have already decided to replace exactly the same function incomit-rs
(comit-network/comit-rs#2330 (comment)), so replacing it here has the nice consequence of not needing to write a new test, since it's not our own code.Side-effects:
TryParams
toswap.ts
since it's directly related and only used for executing a swap.util/timeout_promise.ts
toutil/sleep.ts
, since it now only holds asleep
function.Notes
I don't believe this requires a changelog update. Am I wrong?