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

Implement Reth service module #531

Merged
merged 3 commits into from
Jul 19, 2024
Merged

Conversation

scottbot95
Copy link
Contributor

I tested this against Reth 1.0 (from #530), however I don't think the command line has changed since the current imported version (0.2.0-beta.6) so I opted to implement it against main instead of on top of my other PR.

I chose not to implement and integration test with a nixos vm because I felt it is mostly likely unecessary due to the extensive test coverage within Reth itself.

@scottbot95 scottbot95 marked this pull request as ready for review July 14, 2024 03:33
@aldoborrero
Copy link
Collaborator

@scottbot95 and we will migrate soonish to Reth 1.0, so even if it doesn't work at all, it's not a problem to merge it now.

Copy link
Collaborator

@aldoborrero aldoborrero left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Collaborator

@aldoborrero aldoborrero left a comment

Choose a reason for hiding this comment

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

@scottbot95 only minor thing:

  • Remove mdDoc as those are effectively deprecated.

@scottbot95
Copy link
Contributor Author

Oh I didn't realize mdDoc isn't needed anymore. It got included since I just copied from another module, but I can remove them.

@aldoborrero aldoborrero merged commit d3d8046 into nix-community:main Jul 19, 2024
80 checks passed
@aldoborrero
Copy link
Collaborator

@scottbot95 we will remove them on PR #523

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.

2 participants