----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48049/#review135525 -----------------------------------------------------------
LGTM, just one minor issue where we could benefit from implementing the functionality in `evolve`. src/master/http.cpp (line 1192) <https://reviews.apache.org/r/48049/#comment200525> New line before. We use 2 new lines to separate elements outside classes. src/master/http.cpp (line 1201) <https://reviews.apache.org/r/48049/#comment200520> New line before src/master/http.cpp (lines 1204 - 1214) <https://reviews.apache.org/r/48049/#comment200523> hmm.. Can't you just implement a evolve function that converts from `MasterInfo` to `v1::MasterInfo` and then just use it here? src/master/http.cpp (line 1210) <https://reviews.apache.org/r/48049/#comment200521> New line before src/master/http.cpp (line 1220) <https://reviews.apache.org/r/48049/#comment200526> Delete a extra new line here src/tests/api_tests.cpp (line 179) <https://reviews.apache.org/r/48049/#comment200527> This was already fixed by bbannier in `cb1fbdaa4`. Can you rebase? src/tests/api_tests.cpp (line 181) <https://reviews.apache.org/r/48049/#comment200529> New line here src/tests/api_tests.cpp (line 183) <https://reviews.apache.org/r/48049/#comment200530> Why the extra new line here? src/tests/api_tests.cpp (line 197) <https://reviews.apache.org/r/48049/#comment200533> The `get()` here is redundant. Can you just do: `v1Response->IsInitialized()` and in the other two places in this test. I know the other tests already use it in this file but it would be good to not introduce new ones. I would clean up the rest in another review. src/tests/api_tests.cpp (line 199) <https://reviews.apache.org/r/48049/#comment200532> Can you swap the order of arguments i.e. `master.get()->getMasterInfo().ip())` is the the expected argument. src/tests/api_tests.cpp (line 345) <https://reviews.apache.org/r/48049/#comment200528> Ditto, rebase - Anand Mazumdar On May 30, 2016, 4:38 p.m., Abhishek Dasgupta wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/48049/ > ----------------------------------------------------------- > > (Updated May 30, 2016, 4:38 p.m.) > > > Review request for mesos and Vinod Kone. > > > Bugs: MESOS-5497 > https://issues.apache.org/jira/browse/MESOS-5497 > > > Repository: mesos > > > Description > ------- > > Implemented GET_LEADING_MASTER Call in v1 master API and > made a minor change in the test case for GET_LOGGING_LEVEL. > > > Diffs > ----- > > src/master/http.cpp c8d2f46d9e0ad8a99a6ebffc6a3d5d852cee0616 > src/master/master.hpp eeeccdfdfd296c2a484764e887564f2e065cfd14 > src/tests/api_tests.cpp 1d7a3a9df395805c250148bf6232b00d712c7a05 > > Diff: https://reviews.apache.org/r/48049/diff/ > > > Testing > ------- > > On Ubuntu 16.04: > > sudo GTEST_FILTER="*MasterAPITest*.*" make -j2 check > > > Thanks, > > Abhishek Dasgupta > >
