----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58095/#review174444 -----------------------------------------------------------
I really like the shape this is taking. Just a really minor issue and it will be ready! src/master/http.cpp Lines 3499-3509 (original), 3501-3511 (patched) <https://reviews.apache.org/r/58095/#comment247618> Could you revert these lines to the style: ```c++ Option<type> variable = None(); if (master->attribute.contains(name)) { variable = master->attribute.at(name); } ``` also, note that we don't use the curly braces `{}` for constructors in order to keep consistency (although I must admit there may be some that pass through the review process). Feel free to use them for initialization lists. - Alexander Rojas On May 8, 2017, 9:41 a.m., Jay Guo wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/58095/ > ----------------------------------------------------------- > > (Updated May 8, 2017, 9:41 a.m.) > > > Review request for mesos, Adam B, Alexander Rojas, Benjamin Mahler, and > Michael Park. > > > Bugs: MESOS-4732 and MESOS-7260 > https://issues.apache.org/jira/browse/MESOS-4732 > https://issues.apache.org/jira/browse/MESOS-7260 > > > Repository: mesos > > > Description > ------- > > Refactored `Master::Http::roles` to use `jsonify`. > > > Diffs > ----- > > src/common/http.hpp 93d6088e97c2384f9f6d26e010a501abf2deb43e > src/common/http.cpp 167dce2b9a2d3b68a1df5b4079f701482d34db28 > src/master/http.cpp e2590a17044ac019b24a24629428d4ec8adc0c31 > > > Diff: https://reviews.apache.org/r/58095/diff/5/ > > > Testing > ------- > > no functional changes > make check > > > Thanks, > > Jay Guo > >
