----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49225/#review139506 -----------------------------------------------------------
src/master/http.cpp (lines 1505 - 1506) <https://reviews.apache.org/r/49225/#comment204772> Do we need this explicit check here? I can understand why we introduced/need it in `GET_LEADER` but why here? Bad Copy/Paste? src/master/http.cpp (line 1508) <https://reviews.apache.org/r/49225/#comment204758> Capture by constant reference since it's an alias. src/master/http.cpp (line 1515) <https://reviews.apache.org/r/49225/#comment204759> Capture by constant reference since it's an alias. src/master/http.cpp (line 1675) <https://reviews.apache.org/r/49225/#comment204760> Pass by reference please. src/master/master.hpp (line 1310) <https://reviews.apache.org/r/49225/#comment204761> `const Resources&` src/master/master.hpp (line 1447) <https://reviews.apache.org/r/49225/#comment204762> s/acceptType/contentType src/tests/api_tests.cpp (line 51) <https://reviews.apache.org/r/49225/#comment204763> Alphabetical please. Move it to L49 src/tests/api_tests.cpp (line 504) <https://reviews.apache.org/r/49225/#comment204765> Usually we avoid such one trivial liner comments that hardly serve any purpose/help improve readability of the code. Can you instead modify this comment with a brief overview of what the test _intends_ to do? See similar tests in other files for inspiration. src/tests/api_tests.cpp (line 523) <https://reviews.apache.org/r/49225/#comment204764> Where is `createFrameworkInfo` defined? src/tests/api_tests.cpp (line 545) <https://reviews.apache.org/r/49225/#comment204770> It would be good to use the operator `->` here. Just `offers->size()` should work. Here and in other places in the test. src/tests/api_tests.cpp (line 563) <https://reviews.apache.org/r/49225/#comment204766> Nit: s/mesos::internal/internal Here and in all the other places in this test please. src/tests/api_tests.cpp (lines 567 - 568) <https://reviews.apache.org/r/49225/#comment204771> Do we need the explicit `static_cast` here? Why doesn't `CopyFrom()` work directly? src/tests/api_tests.cpp (line 572) <https://reviews.apache.org/r/49225/#comment204767> Kill this comment src/tests/api_tests.cpp (line 580) <https://reviews.apache.org/r/49225/#comment204768> Kill extra new line here - Anand Mazumdar On June 25, 2016, 6:52 a.m., Abhishek Dasgupta wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/49225/ > ----------------------------------------------------------- > > (Updated June 25, 2016, 6:52 a.m.) > > > Review request for mesos, Anand Mazumdar and Vinod Kone. > > > Bugs: MESOS-5499 > https://issues.apache.org/jira/browse/MESOS-5499 > > > Repository: mesos > > > Description > ------- > > Implemented RESERVE_RESOURCES Call in v1 master API. > > > Diffs > ----- > > src/master/http.cpp 70f084b84db90fde20c05d2354be190f28e72996 > src/master/master.hpp e983d1ba6ebcdaf2ace419201659e53edaa2a0aa > src/tests/api_tests.cpp 7f16f43c3968cd56cf93951489079032093beaeb > > Diff: https://reviews.apache.org/r/49225/diff/ > > > Testing > ------- > > > Thanks, > > Abhishek Dasgupta > >
