Skip to content
This repository has been archived by the owner on Dec 10, 2021. It is now read-only.

Type promises #47

Merged
merged 1 commit into from
Dec 3, 2018
Merged

Type promises #47

merged 1 commit into from
Dec 3, 2018

Conversation

xtinec
Copy link
Contributor

@xtinec xtinec commented Dec 2, 2018

🏆 Enhancements

  • type rejectAfterTimeout with generics.
  • fixing an import typo in tests.

- type rejectAfterTimeout with generics.
- fixing an import typo in tests.
@xtinec xtinec requested a review from a team December 2, 2018 18:03
@codecov
Copy link

codecov bot commented Dec 2, 2018

Codecov Report

Merging #47 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #47   +/-   ##
=======================================
  Coverage   99.28%   99.28%           
=======================================
  Files          57       57           
  Lines         560      560           
  Branches       39       39           
=======================================
  Hits          556      556           
  Misses          2        2           
  Partials        2        2
Impacted Files Coverage Δ
...et-ui-connection/src/callApi/rejectAfterTimeout.ts 100% <ø> (ø) ⬆️
...nnection/src/callApi/callApiAndParseWithTimeout.ts 100% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 96a6487...795e635. Read the comment docs.

Copy link
Contributor

@williaster williaster left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! thanks for fixing the import, wonder why this didn't throw a linting error / fail the build 🤔

also what's the advantage of generics in this situation?

@xtinec
Copy link
Contributor Author

xtinec commented Dec 3, 2018

It lets the caller of rejectAfterTimeout specify a more specific type of the value that is being promised. For example, think using it to limit the return value to string instead of Json.

I'm not sure what happened with our lint settings. Only noticed it as VSCode complained. Will take a closer look at our lint settings.

@xtinec xtinec merged commit 990a463 into master Dec 3, 2018
@delete-merged-branch delete-merged-branch bot deleted the type-promise branch December 3, 2018 19:03
@kristw kristw added #code-quality and removed reviewable Ready for review labels Dec 6, 2018
@kristw kristw added this to the v0.8.0 milestone Feb 1, 2019
kristw pushed a commit that referenced this pull request Apr 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants