----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45922/#review130041 -----------------------------------------------------------
Fix it, then Ship it! Looks great! Just one issue with variable naming (one underscore doesn't bother me much, but two does) docs/configuration.md (lines 937 - 939) <https://reviews.apache.org/r/45922/#comment193654> Unnecessary here; only necessary for the --acls flag. src/slave/main.cpp (line 292) <https://reviews.apache.org/r/45922/#comment193655> You can leave out the quotes. I just wanted the hyphenation src/tests/cluster.cpp (line 392) <https://reviews.apache.org/r/45922/#comment193656> Double underscores for variable names are discouraged (usually only acceptable for continuation function names). Maybe the original parameter is `providedAuthorizer`, and `authorizer__` becomes `newAuthorizer`, then you could have: `authorizer = providedAuthorizer;` `if(...) newAuthorizer = Authorizer::create();` `if(newAuthorizer.isSome()) authorizer = newAuthorizer;` Or maybe you've got better names, but you get the idea. src/slave/main.cpp (lines 291 - 293) <https://reviews.apache.org/r/45922/#comment193668> Let's move this comment into the `if (authorizerName != slave::DEFAULT_AUTHORIZER)` block, since that's the condition it applies to. Then you can even simplify it down to "NOTE: The contents of --acls are ignored for custom authorizers." src/tests/cluster.cpp (lines 389 - 391) <https://reviews.apache.org/r/45922/#comment193669> Simplify and move to before `Authorizer::create(authorizerName);` - Adam B On April 21, 2016, 7:48 a.m., Jan Schlicht wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/45922/ > ----------------------------------------------------------- > > (Updated April 21, 2016, 7:48 a.m.) > > > Review request for mesos, Adam B and Alexander Rojas. > > > Bugs: MESOS-5142 > https://issues.apache.org/jira/browse/MESOS-5142 > > > Repository: mesos > > > Description > ------- > > See summary. > > > Diffs > ----- > > docs/configuration.md 86ba66ac62295ca148524bcb2e57fee560ac4ac5 > src/local/local.cpp 7de8a2423185e49dfa849d069938d3243b4f4956 > src/slave/constants.hpp 9978c11fec40055dd42f19c20cd3e9fef4e78cea > src/slave/flags.hpp ee520acc459564fe68272950948fc80c5e24513a > src/slave/flags.cpp 10d2974bd2b6e79255fc894979607f0d2d00c315 > src/slave/main.cpp 38bd00584dd9c6a872398678b2288edeed1cd2a4 > src/slave/slave.hpp f78c1b4e4d5378ef7223c6eb3ea45491c30fb4c1 > src/slave/slave.cpp d82dec2b10d496065013eb4ad6a35dc054b72553 > src/tests/cluster.hpp 96ec52af16776e91200aa755c7847f56e33d71f4 > src/tests/cluster.cpp 3e5fdf6b32a0d99d3ca83743386232d38471e34f > src/tests/mesos.hpp e4b63d41d883807ac39846799468b80e88c84e0b > src/tests/mesos.cpp b5937af7713e1ee2af475518b3e968b2defa8beb > > Diff: https://reviews.apache.org/r/45922/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Jan Schlicht > >
