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

Add more request transformers #3161

Merged
merged 1 commit into from
Sep 1, 2024

Conversation

lorisleiva
Copy link
Collaborator

@lorisleiva lorisleiva commented Aug 26, 2024

This PR finishes the refactoring of the getDefaultRequestTransformerForSolanaRpc function by creating three new request transformers and delegating to them:

  • getIntegerOverflowRequestTransformer: Wraps the integer overflow visitor in a request transformer.
  • getBigIntDowncastRequestTransformer: Wraps the bigint downcast visitor in a request transformer.
  • getTreeWalkerRequestTransformer: Helper that creates request transformers from visitors.

Additionally, this PR changes the definition of the onIntegerOverflow slightly by using request: RpcRequest as its first argument instead of methodName: string to be more consistent with the rest of the RPC API.

Copy link

changeset-bot bot commented Aug 26, 2024

🦋 Changeset detected

Latest commit: 8fde46a

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

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

Copy link
Collaborator

@buffalojoec buffalojoec left a comment

Choose a reason for hiding this comment

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

I think it would make more sense for this PR to be two separate PRs. You've got a pretty clear distinction between the two in both the changeset and the READMEs.

@lorisleiva
Copy link
Collaborator Author

@buffalojoec Yeah I should have done but because I'm renaming filenames in this PR, running gt split is a nightmare to resolve. 😔

Copy link
Collaborator

@steveluscher steveluscher left a comment

Choose a reason for hiding this comment

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

Added via Giphy

Copy link
Collaborator Author

lorisleiva commented Sep 1, 2024

Merge activity

  • Sep 1, 2:06 PM EDT: @lorisleiva started a stack merge that includes this pull request via Graphite.
  • Sep 1, 2:33 PM EDT: Graphite rebased this pull request as part of a merge.
  • Sep 1, 2:34 PM EDT: @lorisleiva merged this pull request with Graphite.

@lorisleiva lorisleiva changed the base branch from loris/refactor-fix-479 to graphite-base/3161 September 1, 2024 18:29
@lorisleiva lorisleiva changed the base branch from graphite-base/3161 to master September 1, 2024 18:31
@lorisleiva lorisleiva force-pushed the loris/add-more-request-transformers branch from 800d312 to 8fde46a Compare September 1, 2024 18:32
@lorisleiva lorisleiva merged commit 9dfca45 into master Sep 1, 2024
7 checks passed
@lorisleiva lorisleiva deleted the loris/add-more-request-transformers branch September 1, 2024 18:34
Copy link
Contributor

Because there has been no activity on this PR for 14 days since it was merged, it has been automatically locked. Please open a new issue if it requires a follow up.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 16, 2024
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.

3 participants