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

Simplify EthFrame implementation #1886

Open
rakita opened this issue Dec 5, 2024 · 4 comments
Open

Simplify EthFrame implementation #1886

rakita opened this issue Dec 5, 2024 · 4 comments
Labels
good first issue Good for newcomers

Comments

@rakita
Copy link
Member

rakita commented Dec 5, 2024

Frame trait seems great, but implementation for Eth seem bloated. find the way how to make it nicer and more manageable

@rakita rakita added the good first issue Good for newcomers label Dec 5, 2024
@Ayushdubey86
Copy link
Contributor

@rakita , is there a need of different init_first & init? was thinking to refactor this, but it has big impact at handler.rs as well

@rakita
Copy link
Member Author

rakita commented Feb 13, 2025

The first one was for initializing the first frame, IIRC correctly it is to have memory, and the second one is for others. Probably can be done with only one functions but I am not sure how would that look

@Ayushdubey86
Copy link
Contributor

It'll take some time, but I'll look into this

@Ayushdubey86
Copy link
Contributor

Analysis on EthFrame Method:

After reviewing the init and init_first methods, I considered combining them. However, I did not observe any significant performance gains or simplification. In fact, merging them might increase complexity for new contributors.

As an alternative, I suggest renaming the methods to more descriptive names and improving the documentation to enhance clarity. If you have any other insights or spot potential improvements, feel free to share.
@rakita

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants