----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29289/#review74860 -----------------------------------------------------------
3rdparty/libprocess/include/process/address.hpp <https://reviews.apache.org/r/29289/#comment121664> Why change this? I thought the original code should just work fine because `hash_combine` will invoke `hash_value` for `IP`. 3rdparty/libprocess/include/process/address.hpp <https://reviews.apache.org/r/29289/#comment122608> Please follow the style guide for includes: ``` #include <boost/functional/hash.hpp> #include <stout/ip.hpp> ``` 3rdparty/libprocess/include/process/address.hpp <https://reviews.apache.org/r/29289/#comment122609> No need to use leading underscore for all parameters! In this case, it's not necessary. 3rdparty/libprocess/include/process/address.hpp <https://reviews.apache.org/r/29289/#comment122610> Let's not add any IPv6 impl. yet in this patch. Please remove them. 3rdparty/libprocess/include/process/network.hpp <https://reviews.apache.org/r/29289/#comment122638> Why do you want to pass in the family? Can you get the family from sockaddr_storage? This changes the Socket API below which we should avoid if uncesssary. 3rdparty/libprocess/include/process/network.hpp <https://reviews.apache.org/r/29289/#comment122641> I don't think you need to initialize the sockaddr_storage here? As a result, you don't need to construct an IP. 3rdparty/libprocess/include/process/pid.hpp <https://reviews.apache.org/r/29289/#comment122611> Add a blank line above `#include <stout/ip.hpp>` 3rdparty/libprocess/include/process/socket.hpp <https://reviews.apache.org/r/29289/#comment122643> As I explained above, no need to do this API change! 3rdparty/libprocess/src/pid.cpp <https://reviews.apache.org/r/29289/#comment122613> Add a space before `TODO`. 3rdparty/libprocess/src/pid.cpp <https://reviews.apache.org/r/29289/#comment122626> As we discussed offline. This change is not necessary. Please revert. 3rdparty/libprocess/src/process.cpp <https://reviews.apache.org/r/29289/#comment122646> Please do not remove any logging. ``` LOG(FATAL) << "Parsing LIBPROCESS_IP=" << value << " failed: " << ip.error(); ``` `ip.isError()` is a boolean, are you going to print a boolean when the parsing fails? - Jie Yu On March 6, 2015, 5:16 p.m., Evelina Dumitrescu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/29289/ > ----------------------------------------------------------- > > (Updated March 6, 2015, 5:16 p.m.) > > > Review request for mesos, Benjamin Hindman, Dominic Hamon, Jie Yu, Joris Van > Remoortere, and Niklas Nielsen. > > > Bugs: MESOS-1919 > https://issues.apache.org/jira/browse/MESOS-1919 > > > Repository: mesos > > > Description > ------- > > see summary. > > > Diffs > ----- > > 3rdparty/libprocess/3rdparty/Makefile.am > e55147ae535b2b27271b69da3db7aff6cd202368 > 3rdparty/libprocess/include/process/address.hpp > f5fb4dd2fdeaade36390c083bee6f2787df4b6d3 > 3rdparty/libprocess/include/process/network.hpp > 7620278e4b2c376e37353325aa717fe54b819272 > 3rdparty/libprocess/include/process/pid.hpp > 30c466c8defb45bbfaaf68611fb68285a89fea6e > 3rdparty/libprocess/include/process/socket.hpp > 2c4258775bd90f91f1922a3cf2bb4ddf5c334881 > 3rdparty/libprocess/src/http.cpp f9d9daf0bbde4286cb5b47cd121b7513d9199915 > 3rdparty/libprocess/src/pid.cpp d28a1541a10582c0ec42a2c26ee3bdb6683bb8bd > 3rdparty/libprocess/src/poll_socket.cpp > ec781a3ac0a245831c679826c97045fd3a2b4656 > 3rdparty/libprocess/src/process.cpp > 671aff6692be4ec49f7af4851a0511c178e58b45 > 3rdparty/libprocess/src/socket.cpp 0e1cebb19e21c706b152d35a0b8722924c971a35 > 3rdparty/libprocess/src/tests/http_tests.cpp > 0a00ae59fe999344eb05bee04ab3fb703a3265e7 > 3rdparty/libprocess/src/tests/process_tests.cpp > eb38edce2c483ba7f963a826893a15a075238618 > > Diff: https://reviews.apache.org/r/29289/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Evelina Dumitrescu > >
