Re: Review Request 27054: Libprocess Socket: introduce connect()
On Oct. 29, 2014, 4:57 p.m., Mesos ReviewBot wrote: Bad patch! Reviews applied: [27350, 27351, 27350] Failed command: ./support/apply-review.sh -n -r 27350 Error: 2014-10-29 23:57:20 URL:https://reviews.apache.org/r/27350/diff/raw/ [897/897] - 27350.patch [1] error: patch failed: 3rdparty/libprocess/m4/ax_cxx_compile_stdcxx_11.m4:37 error: 3rdparty/libprocess/m4/ax_cxx_compile_stdcxx_11.m4: patch does not apply Failed to apply patch Do you need to rebase? - Niklas --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27054/#review59083 --- On Oct. 29, 2014, 3:42 p.m., Joris Van Remoortere wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27054/ --- (Updated Oct. 29, 2014, 3:42 p.m.) Review request for mesos, Benjamin Hindman and Niklas Nielsen. Bugs: MESOS-1330 https://issues.apache.org/jira/browse/MESOS-1330 Repository: mesos-git Description --- Introduce the connect(const Node node) call to Socket. Use it in link() and send(). Diffs - 3rdparty/libprocess/include/process/socket.hpp 6683881 3rdparty/libprocess/src/process.cpp 85fb995 Diff: https://reviews.apache.org/r/27054/diff/ Testing --- make check Thanks, Joris Van Remoortere
Re: Review Request 27054: Libprocess Socket: introduce connect()
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27054/#review58966 --- Patch looks great! Reviews applied: [27023, 27054] All tests passed. - Mesos ReviewBot On Oct. 29, 2014, 5:18 a.m., Joris Van Remoortere wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27054/ --- (Updated Oct. 29, 2014, 5:18 a.m.) Review request for mesos, Benjamin Hindman and Niklas Nielsen. Bugs: MESOS-1330 https://issues.apache.org/jira/browse/MESOS-1330 Repository: mesos-git Description --- Introduce the connect(const Node node) call to Socket. Use it in link() and send(). Diffs - 3rdparty/libprocess/include/process/socket.hpp 6683881 3rdparty/libprocess/src/process.cpp 85fb995 Diff: https://reviews.apache.org/r/27054/diff/ Testing --- make check Thanks, Joris Van Remoortere
Re: Review Request 27054: Libprocess Socket: introduce connect()
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27054/#review58907
---
3rdparty/libprocess/include/process/socket.hpp
https://reviews.apache.org/r/27054/#comment100075
We name our enumerations like constants, and we name our constants in
capital letters, like macros. See:
http://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Enumerator_Names
(but note that we don't use the 'k' prefix style for naming, which was only
recently accepted for enumerations in the Google Style Guide).
Check out other examples in the code base too.
3rdparty/libprocess/include/process/socket.hpp
https://reviews.apache.org/r/27054/#comment100076
{ on newline please.
3rdparty/libprocess/include/process/socket.hpp
https://reviews.apache.org/r/27054/#comment100078
This should be virtual, no?
3rdparty/libprocess/include/process/socket.hpp
https://reviews.apache.org/r/27054/#comment100147
We've put friend declarations at the top of the private section because, if
you're not a friend, you have no reason to read below that line! ;-)
In all seriousness, the rationale here is exactly the same reasoning for
why we order public, protected, then private: the class declaration acts as
documentation, and most people care about the public members of the class first.
3rdparty/libprocess/src/process.cpp
https://reviews.apache.org/r/27054/#comment100150
I'd like us to call this struct Connect instead of AsyncConnect since this
will not be the first time that we have to create a struct for holding data
across continuations and it would be great to set a precedent for naming
continuation data. That is, this is a fundamental pattern that we'll replicate.
So in the future, we can have an internal::Write which we know maps to a
'write' continuation, etc. Sound good?
3rdparty/libprocess/src/process.cpp
https://reviews.apache.org/r/27054/#comment100151
Let's stay consistent with how we label our continuations:
s/async_receiving_connect/_connect/.
- Benjamin Hindman
On Oct. 29, 2014, 5:18 a.m., Joris Van Remoortere wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27054/
---
(Updated Oct. 29, 2014, 5:18 a.m.)
Review request for mesos, Benjamin Hindman and Niklas Nielsen.
Bugs: MESOS-1330
https://issues.apache.org/jira/browse/MESOS-1330
Repository: mesos-git
Description
---
Introduce the connect(const Node node) call to Socket. Use it in link() and
send().
Diffs
-
3rdparty/libprocess/include/process/socket.hpp 6683881
3rdparty/libprocess/src/process.cpp 85fb995
Diff: https://reviews.apache.org/r/27054/diff/
Testing
---
make check
Thanks,
Joris Van Remoortere
Re: Review Request 27054: Libprocess Socket: introduce connect()
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27054/#review59042 --- 3rdparty/libprocess/include/process/socket.hpp https://reviews.apache.org/r/27054/#comment100324 not without a check in configure, you don't :) 3rdparty/libprocess/include/process/socket.hpp https://reviews.apache.org/r/27054/#comment100325 needs a check in configure. - Dominic Hamon On Oct. 28, 2014, 10:18 p.m., Joris Van Remoortere wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27054/ --- (Updated Oct. 28, 2014, 10:18 p.m.) Review request for mesos, Benjamin Hindman and Niklas Nielsen. Bugs: MESOS-1330 https://issues.apache.org/jira/browse/MESOS-1330 Repository: mesos-git Description --- Introduce the connect(const Node node) call to Socket. Use it in link() and send(). Diffs - 3rdparty/libprocess/include/process/socket.hpp 6683881 3rdparty/libprocess/src/process.cpp 85fb995 Diff: https://reviews.apache.org/r/27054/diff/ Testing --- make check Thanks, Joris Van Remoortere
Re: Review Request 27054: Libprocess Socket: introduce connect()
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27054/ --- (Updated Oct. 29, 2014, 9:47 p.m.) Review request for mesos, Benjamin Hindman and Niklas Nielsen. Changes --- Depends on mutex configure check for libprocess. Bugs: MESOS-1330 https://issues.apache.org/jira/browse/MESOS-1330 Repository: mesos-git Description --- Introduce the connect(const Node node) call to Socket. Use it in link() and send(). Diffs - 3rdparty/libprocess/include/process/socket.hpp 6683881 3rdparty/libprocess/src/process.cpp 85fb995 Diff: https://reviews.apache.org/r/27054/diff/ Testing --- make check Thanks, Joris Van Remoortere
Re: Review Request 27054: Libprocess Socket: introduce connect()
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27054/#review59063 --- Patch looks great! Reviews applied: [27350, 27023, 27054] All tests passed. - Mesos ReviewBot On Oct. 29, 2014, 9:47 p.m., Joris Van Remoortere wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27054/ --- (Updated Oct. 29, 2014, 9:47 p.m.) Review request for mesos, Benjamin Hindman and Niklas Nielsen. Bugs: MESOS-1330 https://issues.apache.org/jira/browse/MESOS-1330 Repository: mesos-git Description --- Introduce the connect(const Node node) call to Socket. Use it in link() and send(). Diffs - 3rdparty/libprocess/include/process/socket.hpp 6683881 3rdparty/libprocess/src/process.cpp 85fb995 Diff: https://reviews.apache.org/r/27054/diff/ Testing --- make check Thanks, Joris Van Remoortere
Re: Review Request 27054: Libprocess Socket: introduce connect()
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27054/ --- (Updated Oct. 29, 2014, 10:42 p.m.) Review request for mesos, Benjamin Hindman and Niklas Nielsen. Changes --- Depends on enable_shared_from_this configure check for libprocess. Bugs: MESOS-1330 https://issues.apache.org/jira/browse/MESOS-1330 Repository: mesos-git Description --- Introduce the connect(const Node node) call to Socket. Use it in link() and send(). Diffs - 3rdparty/libprocess/include/process/socket.hpp 6683881 3rdparty/libprocess/src/process.cpp 85fb995 Diff: https://reviews.apache.org/r/27054/diff/ Testing --- make check Thanks, Joris Van Remoortere
Re: Review Request 27054: Libprocess Socket: introduce connect()
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27054/#review59083 --- Bad patch! Reviews applied: [27350, 27351, 27350] Failed command: ./support/apply-review.sh -n -r 27350 Error: 2014-10-29 23:57:20 URL:https://reviews.apache.org/r/27350/diff/raw/ [897/897] - 27350.patch [1] error: patch failed: 3rdparty/libprocess/m4/ax_cxx_compile_stdcxx_11.m4:37 error: 3rdparty/libprocess/m4/ax_cxx_compile_stdcxx_11.m4: patch does not apply Failed to apply patch - Mesos ReviewBot On Oct. 29, 2014, 10:42 p.m., Joris Van Remoortere wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27054/ --- (Updated Oct. 29, 2014, 10:42 p.m.) Review request for mesos, Benjamin Hindman and Niklas Nielsen. Bugs: MESOS-1330 https://issues.apache.org/jira/browse/MESOS-1330 Repository: mesos-git Description --- Introduce the connect(const Node node) call to Socket. Use it in link() and send(). Diffs - 3rdparty/libprocess/include/process/socket.hpp 6683881 3rdparty/libprocess/src/process.cpp 85fb995 Diff: https://reviews.apache.org/r/27054/diff/ Testing --- make check Thanks, Joris Van Remoortere
Re: Review Request 27054: Libprocess Socket: introduce connect()
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27054/ --- (Updated Oct. 29, 2014, 5:18 a.m.) Review request for mesos, Benjamin Hindman and Niklas Nielsen. Changes --- rebase on 27023 Bugs: MESOS-1330 https://issues.apache.org/jira/browse/MESOS-1330 Repository: mesos-git Description --- Introduce the connect(const Node node) call to Socket. Use it in link() and send(). Diffs (updated) - 3rdparty/libprocess/include/process/socket.hpp 6683881 3rdparty/libprocess/src/process.cpp 85fb995 Diff: https://reviews.apache.org/r/27054/diff/ Testing --- make check Thanks, Joris Van Remoortere
