> On Dec. 8, 2014, 5:52 a.m., Ben Mahler wrote: > > Really appreciate the follow up Evelina, great work! > > > > Just looked at style, since I don't have context, I'll leave a few > > questions higher level questions for other reviewers to address: > > > > (1) Does it makes sense for all of these to be at the top level "process" > > namespace? Seems like now that we're introducing a family of networking > > calls, perhaps they belong inside a "process::network" namespace? > > > > (2) We may want these to be simple wrappers around the syscall and avoid > > Try. Any benefit from checking the error code will be lost when using Try > > around these.
I agree with (1). Because it has been brought into discussion the code refactoring changes from libprocess, I want to suggest some more related to namespace naming: net -> network os -> operating_system fs -> file_system I don't quite agree with (2). In most cases, we do the same check in every callsite for success/failure(eg if error < 0). If we need something more specific, we can figure it out from the errno variable. - Evelina ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28789/#review64199 ----------------------------------------------------------- On Dec. 7, 2014, 1:40 a.m., Evelina Dumitrescu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/28789/ > ----------------------------------------------------------- > > (Updated Dec. 7, 2014, 1:40 a.m.) > > > Review request for mesos, Ben Mahler and Dominic Hamon. > > > Repository: mesos-git > > > Description > ------- > > Revised comment and improved implementation for socket call wrappers > > > Diffs > ----- > > 3rdparty/libprocess/include/process/socket.hpp > 916576f7b901ab57a03395cef403964a050835b1 > 3rdparty/libprocess/src/http.cpp 465e4473685a2e441401e454ad1e8c04db2a4e0a > 3rdparty/libprocess/src/process.cpp > b87ac2206548815bc992c955252567c131fe6a47 > 3rdparty/libprocess/src/tests/http_tests.cpp > 3bda76d4b4b958e956b4f5c248190c2fdfcd0d51 > 3rdparty/libprocess/src/tests/process_tests.cpp > 45be2e15d6f57db89ba7ab758d828ffce3be4b61 > > Diff: https://reviews.apache.org/r/28789/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Evelina Dumitrescu > >
