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.

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

feat: add types to parse streams webhook data #713

Merged
merged 67 commits into from
Oct 25, 2022
Merged

Conversation

ErnoW
Copy link
Member

@ErnoW ErnoW commented Sep 29, 2022


name: 'Pull request'
about: A new pull request

New Pull Request

Checklist

  • I am not disclosing a vulnerability.
  • My code is conform the code style
  • I have made corresponding changes to the documentation
  • I have updated Typescript definitions when needed

Issue Description

Related issue: #FILL_THIS_OUT

Solution Description

@changeset-bot
Copy link

changeset-bot bot commented Sep 29, 2022

⚠️ No Changeset found

Latest commit: 223a545

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@github-actions
Copy link
Contributor

github-actions bot commented Oct 17, 2022

Test coverage

Title Lines Statements Branches Functions
api-utils Coverage: 60%
60.45% (295/488) 44.62% (54/121) 64.48% (69/107)
auth Coverage: 100%
100% (143/143) 90.9% (20/22) 100% (42/42)
core Coverage: 89%
90.23% (573/635) 75.3% (122/162) 84.09% (111/132)
evm-api Coverage: 82%
81.46% (466/572) 16.56% (54/326) 64.3% (191/297)
common-evm-utils Coverage: 62%
63.05% (669/1061) 35.01% (104/297) 30.43% (112/368)
sol-api Coverage: 100%
100% (20/20) 66.66% (4/6) 100% (6/6)
common-streams-utils Coverage: 95%
95.6% (674/705) 97.93% (190/194) 100% (244/244)
streams Coverage: 82%
82.72% (388/469) 64% (64/100) 73.6% (92/125)

@ErnoW ErnoW marked this pull request as ready for review October 17, 2022 17:12
@b4rtaz
Copy link
Collaborator

b4rtaz commented Oct 19, 2022

I wonder, maybe we should move streams types (EvmStream, EvmStreamResult etc) to a new package: @moralisweb3/common-streams. Why common? Because if I need to transfer some value from the back-end side to the front-end side we should create these types as a "common" side.

@ErnoW
Copy link
Member Author

ErnoW commented Oct 19, 2022

I wonder, maybe we should move streams types (EvmStream, EvmStreamResult etc) to a new package: @moralisweb3/common-streams. Why common? Because if I need to transfer some value from the back-end side to the front-end side we should create these types as a "common" side.

Interesting point. Do you think there will be any use-cases to use these types directly in the client? I think that this feature is mostly backend-only, and that the frontend will used partial data from this, mainly via specific endpoints/database lookups.

If there is no use-case then I'd prefer to keep it simple and contained in the streams package.

b4rtaz
b4rtaz previously approved these changes Oct 25, 2022
@ErnoW ErnoW changed the base branch from main to beta October 25, 2022 12:55
@ErnoW ErnoW dismissed b4rtaz’s stale review October 25, 2022 12:55

The base branch was changed.

@ErnoW ErnoW merged commit 69459ee into beta Oct 25, 2022
@ErnoW ErnoW deleted the parse-streams-data branch October 25, 2022 14:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants