Skip to content
This repository has been archived by the owner on Sep 11, 2020. It is now read-only.

Replace timeout function with external dependency #179

Merged
merged 1 commit into from
Mar 31, 2020

Conversation

luckysori
Copy link
Collaborator

Fixes #120 (imo).

Straight from the commit

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.

Notes

I don't believe this requires a changelog update. Am I wrong?

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.
@mergify
Copy link
Contributor

mergify bot commented Mar 31, 2020

Are you sure the changelog does not need updating?

2 similar comments
@mergify
Copy link
Contributor

mergify bot commented Mar 31, 2020

Are you sure the changelog does not need updating?

@mergify
Copy link
Contributor

mergify bot commented Mar 31, 2020

Are you sure the changelog does not need updating?

Copy link
Member

@da-kami da-kami left a 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.

@da-kami da-kami added the no-mergify Stop mergify to merge this automatically label Mar 31, 2020
@luckysori
Copy link
Collaborator Author

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.

@luckysori
Copy link
Collaborator Author

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.

create-comit-app tests pass with these changes.

@luckysori luckysori removed the no-mergify Stop mergify to merge this automatically label Mar 31, 2020
@mergify
Copy link
Contributor

mergify bot commented Mar 31, 2020

bors r+

@bors
Copy link
Contributor

bors bot commented Mar 31, 2020

Build succeeded

@bors bors bot merged commit cef77e4 into master Mar 31, 2020
@mergify mergify bot deleted the 120-less-code-to-test branch March 31, 2020 22:20
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Test tryExecuteSirenAction
3 participants