----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49301/#review139814 -----------------------------------------------------------
LGTM minus a few minor issues around style/code comments. src/files/files.hpp (lines 53 - 55) <https://reviews.apache.org/r/49301/#comment205176> hmm .. `FilesError` can be used by more than master/agent actors. How about adding a more generic comment on the use case of this error class e.g., something like: ```cpp // Represents the various errors that can be returned by methods on the `Files` class via a `Try` that has failed. ``` src/files/files.hpp (line 61) <https://reviews.apache.org/r/49301/#comment205177> Missing period at the end. Ditto for all the other 3 occurences. s/Request arguments are invalid/Invalid argument src/files/files.hpp (line 62) <https://reviews.apache.org/r/49301/#comment205178> s/could not be/not src/files/files.hpp (line 71) <https://reviews.apache.org/r/49301/#comment205164> `const std::string&` Also, no need for the `explicit` keyword on a multi arg constructor. Though, C++11 allows you to use the brace initialization syntax, AFAICT, we don't use this widely in our code yet and would be inconsistent. src/files/files.hpp (lines 72 - 74) <https://reviews.apache.org/r/49301/#comment205165> Should fit in one line, no? - Anand Mazumdar On June 28, 2016, 11:13 a.m., zhou xing wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/49301/ > ----------------------------------------------------------- > > (Updated June 28, 2016, 11:13 a.m.) > > > Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone. > > > Bugs: mesos-5515 > https://issues.apache.org/jira/browse/mesos-5515 > > > Repository: mesos > > > Description > ------- > > Added FilesError class to identify the errors happened during > the handling of files relevant requests. FilesError is a type > deriving from Error that the master/agent can use to construct > the response. > > > Diffs > ----- > > src/files/files.hpp b767d5bc5bee16e3bd98199773a6bc7d30c1c32d > > Diff: https://reviews.apache.org/r/49301/diff/ > > > Testing > ------- > > make > make check > > > Thanks, > > zhou xing > >
