Skip to content
This repository was archived by the owner on Apr 4, 2024. It is now read-only.

fix batch requests with ws#1326

Closed
baabeetaa wants to merge 4 commits intoevmos:mainfrom
notional-labs:fix-batch-requests-ws
Closed

fix batch requests with ws#1326
baabeetaa wants to merge 4 commits intoevmos:mainfrom
notional-labs:fix-batch-requests-ws

Conversation

@baabeetaa
Copy link
Contributor

Description

Batch Request is in spec of jsonrpc https://www.jsonrpc.org/specification#batch

This PR is a fix to make Batch Request working for websocket.


For contributor use:

  • Targeted PR against correct branch (see CONTRIBUTING.md)
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
  • Code follows the module structure standards.
  • Wrote unit and integration tests
  • Updated relevant documentation (docs/) or specification (x/<module>/spec/)
  • Added relevant godoc comments.
  • Added a relevant changelog entry to the Unreleased section in CHANGELOG.md
  • Re-reviewed Files changed in the Github PR explorer

For admin use:

  • Added appropriate labels to PR (ex. WIP, R4R, docs, etc)
  • Reviewers assigned
  • Squashed all commits, uses message "Merge pull request #XYZ: [title]" (coding standards)

Copy link
Contributor

@fedekunze fedekunze left a comment

Choose a reason for hiding this comment

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

Minor comments, can you also add tests?

@baabeetaa
Copy link
Contributor Author

Minor comments, can you also add tests?

do you mean websocket tests?

@danburck danburck requested a review from adisaran64 September 9, 2022 10:38
Copy link
Contributor

@fedekunze fedekunze left a comment

Choose a reason for hiding this comment

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

ACK, would be great to add websocket tests

@fedekunze
Copy link
Contributor

@baabeetaa can you update the PR?

Copy link
Contributor

@adisaran64 adisaran64 left a comment

Choose a reason for hiding this comment

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

@baabeetaa will add tests with this ticket, can update and merge this PR when possible

@baabeetaa baabeetaa requested a review from a team as a code owner September 14, 2022 17:23
@baabeetaa baabeetaa mentioned this pull request Sep 15, 2022
11 tasks
@fedekunze fedekunze added this to the v0.20.x milestone Sep 16, 2022
@baabeetaa
Copy link
Contributor Author

i'll close this and update changes to #1342 with tests.

@baabeetaa baabeetaa closed this Sep 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

No open projects
Status: Canceled

Development

Successfully merging this pull request may close these issues.

3 participants