> On Feb. 10, 2017, 2:42 a.m., Vinod Kone wrote: > > src/tests/check_tests.cpp > > Lines 177 (patched) > > <https://reviews.apache.org/r/56213/diff/3/?file=1627707#file1627707line177> > > > > s/agent/slave/ > > > > here and everywhere else. > > > > we are not doing this change yet. > > Gastón Kleiman wrote: > We do this in `health_check_tests.cpp`: > > ``` > $ grep "agent = " health_check_tests.cpp | wc -l > 15 > $ grep "slave = " health_check_tests.cpp | wc -l > 0 > $ > ``` > > Vinod Kone wrote: > I think we should fix `health_check_tests.cpp` as well then. > > Gastón Kleiman wrote: > I used `health_check_tests.cpp` just as an example, but we're using this > in a lot of places: > > ``` > $ git grep "agent[^ ]* =" | cut -d: -f1 | sort | uniq > api_tests.cpp > authentication_tests.cpp > containerizer/cgroups_isolator_tests.cpp > containerizer/docker_containerizer_tests.cpp > containerizer/io_switchboard_tests.cpp > containerizer/memory_pressure_tests.cpp > fault_tolerance_tests.cpp > health_check_tests.cpp > hierarchical_allocator_tests.cpp > hook_tests.cpp > master_allocator_tests.cpp > master_tests.cpp > master_validation_tests.cpp > mesos.cpp > mesos.hpp > metrics_tests.cpp > oversubscription_tests.cpp > partition_tests.cpp > reconciliation_tests.cpp > reservation_endpoints_tests.cpp > reservation_tests.cpp > slave_authorization_tests.cpp > slave_recovery_tests.cpp > slave_tests.cpp > sorter_tests.cpp > $ git grep "agent[^ ]* =" | wc -l > 207 > $ > ``` > > Alexander Rukletsov wrote: > The policy I'm sticking to is to use `agent` in all new or being modified > non-user facing code. I'd suggest we keep `agent` here and update in other > places as we go. > > Vinod Kone wrote: > I spot checked some of these files, and looks like the code has been > updated recently to rename `slaveFlags` to `agentFlags` (which I suspect is > what the grep above is capturing) which is unfortunate. I think this arose > from a single commit that Alex Clemmer did. > > It's my fault to not clearly articulate on dev list how phase 2 of slave > to agent rename should be done. But the policy that I've been suggesting to > my reviwees, is to not make a file locally inconsistent. IOW, if the rest of > the file is calling it `agent` use `agent`, otherwise stick to `slave`. I > think this applies to any such change not just slave to agent rename. The > reason being, a new dev coming to a file and adding a new test, would be very > confused if a file has a mix of old and new style. > > So, if you really want to use `agent` here, I recommend to do a sweep to > update this file to do it across all tests (in a dependent review of course). > > Does that make sense?
This is a new file and it only uses `agent*` for variable names. Hence I suggest we keep it as is without renaming everything back to `slave*`. - Alexander ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56213/#review165049 ----------------------------------------------------------- On March 16, 2017, 4:46 p.m., Alexander Rukletsov wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/56213/ > ----------------------------------------------------------- > > (Updated March 16, 2017, 4:46 p.m.) > > > Review request for mesos, Gastón Kleiman and Vinod Kone. > > > Bugs: MESOS-6906 > https://issues.apache.org/jira/browse/MESOS-6906 > > > Repository: mesos > > > Description > ------- > > See summary. > > > Diffs > ----- > > src/tests/check_tests.cpp f035c16920deaf559420ae0d7d881330ff65ae44 > src/tests/mesos.hpp f39e243c2c11bc1c9c757049fda2122727d1fef9 > > > Diff: https://reviews.apache.org/r/56213/diff/7/ > > > Testing > ------- > > See https://reviews.apache.org/r/56218/ > > > Thanks, > > Alexander Rukletsov > >
