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

proto: Refactor Endpoint.handle #2111

Closed
wants to merge 11 commits into from

Conversation

gretchenfrage
Copy link
Collaborator

@gretchenfrage gretchenfrage commented Dec 22, 2024

Just a draft for now.


Main motivation:

I think Endpoint.handle is too long. It's 173 lines and it contains section divider comments like this:

//
// Potentially create a new connection
//

The general architecture of this function dates back to a very long time ago and it used to be a lot smaller. I find it difficult to read and think it should be split up more.

Also: take opportunities to reduce LOC where found. However, some overhead is to be expected due to additional function signatures.

Endpoint::handle is still pretty long, and handle_first_packet is still
reasonably small. We can make balance it out more by moving more logic
into handle_first_packet.
Now that handle has been made much shorter, it's less necessary to have
these large and distinctive section-dividing comments.
Endpoint.stateless_reset is called in 2 places, and both of them
immediately wrap it in DatagramEvent::Response. This commit makes
the stateless_reset function itself do that.
Lets us delete 2 other params.
@gretchenfrage
Copy link
Collaborator Author

Opting to split off into more PRs instead.

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.

1 participant