Review Request 25847: Refactor Libprocess: class Node
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25847/ --- Review request for mesos and Niklas Nielsen. Repository: mesos-git Description --- Move class Node out of process.cpp and into its own header. Part of refactoring process.cpp. Diffs - 3rdparty/libprocess/include/Makefile.am 09f6e41 3rdparty/libprocess/include/process/node.hpp PRE-CREATION 3rdparty/libprocess/src/process.cpp 3ac56c7 Diff: https://reviews.apache.org/r/25847/diff/ Testing --- make check in libprocess support/mesos-stype.py Thanks, Joris Van Remoortere
Re: Review Request 25847: Refactor Libprocess: class Node
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25847/#review54018 --- 3rdparty/libprocess/include/process/node.hpp https://reviews.apache.org/r/25847/#comment93890 worth renaming to IpPort? or Host? 3rdparty/libprocess/include/process/node.hpp https://reviews.apache.org/r/25847/#comment93888 uint64_t? start getting ipv6 support in... ;) - Dominic Hamon On Sept. 19, 2014, 1:31 p.m., Joris Van Remoortere wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25847/ --- (Updated Sept. 19, 2014, 1:31 p.m.) Review request for mesos and Niklas Nielsen. Repository: mesos-git Description --- Move class Node out of process.cpp and into its own header. Part of refactoring process.cpp. Diffs - 3rdparty/libprocess/include/Makefile.am 09f6e41 3rdparty/libprocess/include/process/node.hpp PRE-CREATION 3rdparty/libprocess/src/process.cpp 3ac56c7 Diff: https://reviews.apache.org/r/25847/diff/ Testing --- make check in libprocess support/mesos-stype.py Thanks, Joris Van Remoortere
Re: Review Request 25847: Refactor Libprocess: class Node
On Sept. 19, 2014, 8:32 p.m., Dominic Hamon wrote: 3rdparty/libprocess/include/process/node.hpp, line 32 https://reviews.apache.org/r/25847/diff/1/?file=697014#file697014line32 uint64_t? start getting ipv6 support in... ;) Still won't fit in ipv6 though :( - Nikita --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25847/#review54018 --- On Sept. 19, 2014, 8:31 p.m., Joris Van Remoortere wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25847/ --- (Updated Sept. 19, 2014, 8:31 p.m.) Review request for mesos and Niklas Nielsen. Repository: mesos-git Description --- Move class Node out of process.cpp and into its own header. Part of refactoring process.cpp. Diffs - 3rdparty/libprocess/include/Makefile.am 09f6e41 3rdparty/libprocess/include/process/node.hpp PRE-CREATION 3rdparty/libprocess/src/process.cpp 3ac56c7 Diff: https://reviews.apache.org/r/25847/diff/ Testing --- make check in libprocess support/mesos-stype.py Thanks, Joris Van Remoortere
Re: Review Request 25847: Refactor Libprocess: class Node
On Sept. 19, 2014, 8:32 p.m., Dominic Hamon wrote: 3rdparty/libprocess/include/process/node.hpp, line 32 https://reviews.apache.org/r/25847/diff/1/?file=697014#file697014line32 uint64_t? start getting ipv6 support in... ;) Nikita Vetoshkin wrote: Still won't fit in ipv6 though :( We can use code__uint128_t/code ; however, I am not sure if this is supported accross all our required compilers. The larger issue is that the uint32_t type is currently baked into the protobufs, so this would be a larger compatibility change. - Joris --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25847/#review54018 --- On Sept. 19, 2014, 8:31 p.m., Joris Van Remoortere wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25847/ --- (Updated Sept. 19, 2014, 8:31 p.m.) Review request for mesos and Niklas Nielsen. Repository: mesos-git Description --- Move class Node out of process.cpp and into its own header. Part of refactoring process.cpp. Diffs - 3rdparty/libprocess/include/Makefile.am 09f6e41 3rdparty/libprocess/include/process/node.hpp PRE-CREATION 3rdparty/libprocess/src/process.cpp 3ac56c7 Diff: https://reviews.apache.org/r/25847/diff/ Testing --- make check in libprocess support/mesos-stype.py Thanks, Joris Van Remoortere
Re: Review Request 25847: Refactor Libprocess: class Node
On Sept. 19, 2014, 1:32 p.m., Dominic Hamon wrote: 3rdparty/libprocess/include/process/node.hpp, line 32 https://reviews.apache.org/r/25847/diff/1/?file=697014#file697014line32 uint64_t? start getting ipv6 support in... ;) Nikita Vetoshkin wrote: Still won't fit in ipv6 though :( Joris Van Remoortere wrote: We can use code__uint128_t/code ; however, I am not sure if this is supported accross all our required compilers. The larger issue is that the uint32_t type is currently baked into the protobufs, so this would be a larger compatibility change. I would suggest to post-pone the ipv6 concerns to a full patch which supports it (and tests it). On Sept. 19, 2014, 1:32 p.m., Dominic Hamon wrote: 3rdparty/libprocess/include/process/node.hpp, line 11 https://reviews.apache.org/r/25847/diff/1/?file=697014#file697014line11 worth renaming to IpPort? or Host? With several concepts (including this) being inspired by Erlang conventions, I wouldn't change 'Node': http://www.erlang.org/doc/reference_manual/distributed.html - Niklas --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25847/#review54018 --- On Sept. 19, 2014, 1:31 p.m., Joris Van Remoortere wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25847/ --- (Updated Sept. 19, 2014, 1:31 p.m.) Review request for mesos and Niklas Nielsen. Repository: mesos-git Description --- Move class Node out of process.cpp and into its own header. Part of refactoring process.cpp. Diffs - 3rdparty/libprocess/include/Makefile.am 09f6e41 3rdparty/libprocess/include/process/node.hpp PRE-CREATION 3rdparty/libprocess/src/process.cpp 3ac56c7 Diff: https://reviews.apache.org/r/25847/diff/ Testing --- make check in libprocess support/mesos-stype.py Thanks, Joris Van Remoortere
Re: Review Request 25847: Refactor Libprocess: class Node
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25847/#review54030 --- Ship it! Can you motivate it a bit in context of eventually reworking the libprocess transport/event: https://issues.apache.org/jira/browse/MESOS-1330 and down-sizing the 4000+ line process.cpp :-) 3rdparty/libprocess/include/process/node.hpp https://reviews.apache.org/r/25847/#comment93927 Know it is not yours, but these fit on a single line. Thanks! :) We try to use postfix underscore now to avoid clashes, https://www.gnu.org/software/libc/manual/html_node/Reserved-Names.html I will leave that up to you whether you want to address it here. - Niklas Nielsen On Sept. 19, 2014, 1:31 p.m., Joris Van Remoortere wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25847/ --- (Updated Sept. 19, 2014, 1:31 p.m.) Review request for mesos and Niklas Nielsen. Repository: mesos-git Description --- Move class Node out of process.cpp and into its own header. Part of refactoring process.cpp. Diffs - 3rdparty/libprocess/include/Makefile.am 09f6e41 3rdparty/libprocess/include/process/node.hpp PRE-CREATION 3rdparty/libprocess/src/process.cpp 3ac56c7 Diff: https://reviews.apache.org/r/25847/diff/ Testing --- make check in libprocess support/mesos-stype.py Thanks, Joris Van Remoortere
Re: Review Request 25847: Refactor Libprocess: class Node
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25847/ --- (Updated Sept. 19, 2014, 10:19 p.m.) Review request for mesos and Niklas Nielsen. Changes --- fix style issue. Repository: mesos-git Description --- Move class Node out of process.cpp and into its own header. Part of refactoring process.cpp. Diffs (updated) - 3rdparty/libprocess/include/Makefile.am 09f6e41 3rdparty/libprocess/include/process/node.hpp PRE-CREATION 3rdparty/libprocess/src/process.cpp 3ac56c7 Diff: https://reviews.apache.org/r/25847/diff/ Testing --- make check in libprocess support/mesos-stype.py Thanks, Joris Van Remoortere
Re: Review Request 25847: Refactor Libprocess: class Node
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25847/#review54050 --- Patch looks great! Reviews applied: [25847] All tests passed. - Mesos ReviewBot On Sept. 19, 2014, 10:19 p.m., Joris Van Remoortere wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25847/ --- (Updated Sept. 19, 2014, 10:19 p.m.) Review request for mesos and Niklas Nielsen. Repository: mesos-git Description --- Move class Node out of process.cpp and into its own header. Part of refactoring process.cpp. Diffs - 3rdparty/libprocess/include/Makefile.am 09f6e41 3rdparty/libprocess/include/process/node.hpp PRE-CREATION 3rdparty/libprocess/src/process.cpp 3ac56c7 Diff: https://reviews.apache.org/r/25847/diff/ Testing --- make check in libprocess support/mesos-stype.py Thanks, Joris Van Remoortere
