-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Factor out commonality between WorkerProto::Basic{Client,Server}Connection #11125
Conversation
ff96f7d
to
936fe81
Compare
WorkerProto::BasicServerConnection conn; | ||
conn.to = std::move(to); | ||
conn.from = std::move(from); | ||
conn.protoVersion = clientVersion; |
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.
Can this use designated initializer syntax? I think so
WorkerProto::BasicServerConnection conn; | |
conn.to = std::move(to); | |
conn.from = std::move(from); | |
conn.protoVersion = clientVersion; | |
WorkerProto::BasicServerConnection conn {{ | |
.to = to, | |
.from = from, | |
.protoVersion = clientVersion, | |
}}; |
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.
No, that doesn't work with inheritance.
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.
Just two little nits, very nice on the whole!
Refactors, I like them :D
…ction This also renames clientVersion and daemonVersion to the more correct protoVersion (since it's the version agreed to by both sides).
This wasn't moving the underlying buffer, so if the buffer was non-empty, it could lose data.
decd1cc
to
fa7aa03
Compare
Motivation
This gets rid of some duplicate code. It also renames
clientVersion
anddaemonVersion
to the more correctprotoVersion
(since it's the version agreed to by both sides).This is in preparation of another PR that adds more fine-grained protocol negotiation.
This also fixes a bug in
FdSource::operator =
where it wouldn't move the underlyingBufferedSource
, so it could lose data if the buffer wasn't empty.Context
Priorities and Process
Add 👍 to pull requests you find important.
The Nix maintainer team uses a GitHub project board to schedule and track reviews.