Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Abstract error handling for addresses into AbstractService #842

Merged
merged 3 commits into from
Feb 16, 2022
Merged

Abstract error handling for addresses into AbstractService #842

merged 3 commits into from
Feb 16, 2022

Conversation

0xslipk
Copy link
Contributor

@0xslipk 0xslipk commented Feb 10, 2022

No description provided.

@0xslipk 0xslipk added the I8 - Enhancement Additional feature request label Feb 10, 2022
@0xslipk 0xslipk requested a review from a team as a code owner February 10, 2022 17:59
@0xslipk 0xslipk linked an issue Feb 10, 2022 that may be closed by this pull request
Abstract error handling for addresses into AbstractService #823
Closed
@TarikGul
Copy link
Member

Looks like the following test is failing because this mock data used in the historicApi should be returning a resolved promise i.e Promise<Foo> (similar to the other mock functions in the file). If you could update that, that would be awesome, should fix the tests as well.

@TarikGul
Copy link
Member

closes: #823

TarikGul
TarikGul approved these changes Feb 11, 2022
View reviewed changes
Copy link
Member

@TarikGul TarikGul left a comment

Choose a reason for hiding this comment

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

Overall, lgtm 👍 .

  1. the Docs need to be built using yarn build:docs,

  2. The one test needs to pass (solution written above)

@0xslipk 0xslipk requested a review from a team as a code owner February 16, 2022 13:12
@TarikGul
Copy link
Member

Nice updates 👍

alvicsam
alvicsam approved these changes Feb 16, 2022
View reviewed changes
Copy link
Contributor

@alvicsam alvicsam left a comment

Choose a reason for hiding this comment

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

GHA looks fine, other things I can't review :)

@0xslipk 0xslipk merged commit 88e176b into master Feb 16, 2022
@0xslipk 0xslipk deleted the abstract-error-handling branch February 16, 2022 17:54
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alvicsam alvicsam alvicsam approved these changes

@TarikGul TarikGul TarikGul approved these changes

Assignees
No one assigned
Labels
I8 - Enhancement Additional feature request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Abstract error handling for addresses into AbstractService
3 participants
@0xslipk @TarikGul @alvicsam