----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68146/#review207349 -----------------------------------------------------------
Fix it, then Ship it! include/mesos/authorizer/acls.proto Lines 497 (patched) <https://reviews.apache.org/r/68146/#comment290702> s/`to remove resource providers`/`to mark resource providers as gone`/ include/mesos/authorizer/acls.proto Lines 655 (patched) <https://reviews.apache.org/r/68146/#comment290704> Any reason to choose this order? It seems more natural to put the two resource provider ACLs together, like the order you use in the following switch cases. src/authorizer/local/authorizer.cpp Line 1565 (original), 1580 (patched) <https://reviews.apache.org/r/68146/#comment290705> Move this upward? src/authorizer/local/authorizer.cpp Line 1766 (original), 1789 (patched) <https://reviews.apache.org/r/68146/#comment290706> Make the order consistent? - Chun-Hung Hsiao On Aug. 15, 2018, 1:53 p.m., Benjamin Bannier wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/68146/ > ----------------------------------------------------------- > > (Updated Aug. 15, 2018, 1:53 p.m.) > > > Review request for mesos, Chun-Hung Hsiao and Jan Schlicht. > > > Bugs: MESOS-8403 > https://issues.apache.org/jira/browse/MESOS-8403 > > > Repository: mesos > > > Description > ------- > > Added actions and ACLs to authorize removal of resource providers. > > > Diffs > ----- > > include/mesos/authorizer/acls.proto > f5d2580c29df5c9917a5ce0d5df876b3511438df > include/mesos/authorizer/authorizer.proto > 7330416b44ea12009362c1aae7935b079822efe1 > src/authorizer/local/authorizer.cpp > f99b88e10df1e0959f1ddd2e45374862c2dc0a5b > src/tests/authorization_tests.cpp de57fc97addee4bab9eafae6109a72cbb0c2f4ce > > > Diff: https://reviews.apache.org/r/68146/diff/3/ > > > Testing > ------- > > `make check` > > Additional testing with the test case added in > https://reviews.apache.org/r/68147/. > > > Thanks, > > Benjamin Bannier > >