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

Integrate Request evm env filling in ethapi #9358

Merged
merged 8 commits into from
Jul 9, 2024
Merged

Integrate Request evm env filling in ethapi #9358

merged 8 commits into from
Jul 9, 2024

Conversation

nkysg
Copy link
Contributor

@nkysg nkysg commented Jul 7, 2024 •
edited
Loading

Fix #9296

@nkysg nkysg requested review from mattsse, Rjected and emhane as code owners July 7, 2024 09:34
mattsse
mattsse previously requested changes Jul 8, 2024
View reviewed changes
Copy link
Collaborator

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

thanks, this is almost there, only have pedantic nit

crates/rpc/rpc-eth-api/src/helpers/call.rs Outdated Show resolved Hide resolved
crates/rpc/rpc-eth-api/src/helpers/call.rs Show resolved Hide resolved
@mattsse mattsse added C-enhancement New feature or request A-rpc Related to the RPC implementation labels Jul 8, 2024
@nkysg nkysg requested a review from mattsse July 8, 2024 14:50
emhane
emhane requested changes Jul 9, 2024
View reviewed changes
crates/rpc/rpc-eth-api/src/helpers/call.rs Outdated Show resolved Hide resolved
emhane
emhane requested changes Jul 9, 2024
View reviewed changes
crates/rpc/rpc-eth-api/src/helpers/call.rs Outdated Show resolved Hide resolved
@emhane
Copy link
Member

emhane commented Jul 9, 2024

excuse me, merging main looked like it would merge cleanly but some deps have changed in #9391, let me know if you want help fixing the merge conflicts

@nkysg
Copy link
Contributor Author

nkysg commented Jul 9, 2024

excuse me, merging main looked like it would merge cleanly but some deps have changed in #9391, let me know if you want help fixing the merge conflicts

Let me fix it.

@nkysg nkysg force-pushed the evm_env_filling_ethapi branch from 91eeb68 to 6949fe4 Compare July 9, 2024 14:57
@emhane emhane dismissed mattsse’s stale review July 9, 2024 15:21

changes addressed

@emhane emhane enabled auto-merge July 9, 2024 15:21
@emhane
Copy link
Member

emhane commented Jul 9, 2024

remains to fix docs. check out the Makefile, it contains some helpful commands like make lint and make rustdocs

auto-merge was automatically disabled July 9, 2024 15:22

Head branch was pushed to by a user without write access

@nkysg nkysg force-pushed the evm_env_filling_ethapi branch from f6a7a59 to b097706 Compare July 9, 2024 15:28
mattsse
mattsse approved these changes Jul 9, 2024
View reviewed changes
Copy link
Collaborator

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

cool, this now unblocks further op api separation

@nkysg nkysg requested a review from emhane July 9, 2024 15:42
@nkysg
Copy link
Contributor Author

nkysg commented Jul 9, 2024

@mattsse @emhane Thanks for your help!

@emhane
Copy link
Member

emhane commented Jul 9, 2024

ur very welcome @nkysg , thanks for the appreciation, I believe you can learn a whole lot from contributing to reth 😎

@emhane emhane enabled auto-merge July 9, 2024 15:57
@emhane emhane added this pull request to the merge queue Jul 9, 2024
Merged via the queue into paradigmxyz:main with commit 2fa37b1 Jul 9, 2024
32 checks passed
@nkysg nkysg deleted the evm_env_filling_ethapi branch July 9, 2024 16:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mattsse mattsse mattsse approved these changes

@emhane emhane emhane approved these changes

@Rjected Rjected Awaiting requested review from Rjected Rjected is a code owner

Assignees
No one assigned
Labels
A-rpc Related to the RPC implementation C-enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Integrate Request evm env filling in ethapi
3 participants
@nkysg @emhane @mattsse