> On Dec. 7, 2014, 9:52 p.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. > > Evelina Dumitrescu wrote: > 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.
i'd rather not change those other namespaces. The short names are unobtrusive and (i think) understandable. i agree with evelina on (2). The whole point of a wrapper is to abstract away the details of the system call and use our existing error handling primitives to communicate errors up the stack. - Dominic ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28789/#review64199 ----------------------------------------------------------- On Dec. 6, 2014, 5:40 p.m., Evelina Dumitrescu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/28789/ > ----------------------------------------------------------- > > (Updated Dec. 6, 2014, 5:40 p.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 > >
