----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49394/#review140130 -----------------------------------------------------------
Left a few comments. You introduced an `ErrorFlags::Type` enum which currently can only be in a single state. Will you expand this later? Otherwise I am not sure this is as simple as it can be. src/master/http.cpp (line 1440) <https://reviews.apache.org/r/49394/#comment205439> Please explicitly include `errorbase.hpp`. src/master/http.cpp (line 1443) <https://reviews.apache.org/r/49394/#comment205433> Make this an `enum class` to curb implicit conversions to integer types. src/master/http.cpp (line 1446) <https://reviews.apache.org/r/49394/#comment205438> `UNKNOWN` appears unused. Not sure why you added it. src/master/http.cpp (line 1449) <https://reviews.apache.org/r/49394/#comment205435> Make this `explicit`. src/master/http.cpp (line 1453) <https://reviews.apache.org/r/49394/#comment205436> Please provide conversions of `Type` for `stringify`. Currently and until you use an `enum class` this will just output some integer value which is not super useful. src/master/http.cpp (lines 1455 - 1456) <https://reviews.apache.org/r/49394/#comment205434> Please make this `const` (the base class does the same). src/master/http.cpp (line 1490) <https://reviews.apache.org/r/49394/#comment205432> Since you already used an enum for `Type` you could use a `switch` here so we can catch unhandled cases at compile time. src/master/http.cpp (line 1561) <https://reviews.apache.org/r/49394/#comment205437> `switch`. - Benjamin Bannier On June 30, 2016, 11:49 a.m., Alexander Rojas wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/49394/ > ----------------------------------------------------------- > > (Updated June 30, 2016, 11:49 a.m.) > > > Review request for mesos, Adam B and Vinod Kone. > > > Repository: mesos > > > Description > ------- > > Adds an intermediate function `Master::http::_flags()` which performs > authorization and it is called by both the endpoint `/flags` handler > and the HTTP API v1 call `flags` handler. > > > Diffs > ----- > > src/master/http.cpp e5acdb8e0bbcd7a2b7e8a8bc7f4bbeaae2c4fea1 > src/master/master.hpp e2ab2110fe5a287ab16ac9ef4222fed633e02ebe > > Diff: https://reviews.apache.org/r/49394/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Alexander Rojas > >
