-
Notifications
You must be signed in to change notification settings - Fork 8
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
Introduce TCP API #433
base: main
Are you sure you want to change the base?
Introduce TCP API #433
Conversation
101cf70
to
d1d7041
Compare
This is a pretty huge review! It's going to be pretty tough to do a solid review on this and keep all of the details in my head at once. That not only will make it tricky to give/receive high quality review feedback with fast turnaround time, but it also increases the chance that this review lasts for a long time which could make it pretty difficult to get through. Can we think about how to break this up into multiple smaller reviews that we can review and merge incrementally? (Here's a "why" guide for small PRs, but it doesn't touch much on the "how": https://google.github.io/eng-practices/review/developer/small-cls.html ) I haven't dug into this deeply enough to figure out: are there standalone files that can be reviewed independently that we could start with? Or some initial classes or function we could add first? Context for me as a reviewer: I'm not familiar with the details of TCP, so it's even more valuable than normal to have small/digestible reviews since I'll need an unusual amount of time to gather the domain knowledge I'll need to review this properly. One quick structural question: I see lots of files starting with the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Moving myself off of the action list while we think about ways to break this PR up into more reviewable chunks)
d1d7041
to
7550141
Compare
7550141
to
833249b
Compare
@benh, I've changed the behavior of our EventLoop. Now it is running The only part that I'm not really sure about is the part with |
ee19898
to
63beb98
Compare
6acbfa5
to
d51f568
Compare
d51f568
to
25e6b44
Compare
TODO: