----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43824/#review122622 -----------------------------------------------------------
src/tests/hierarchical_allocator_tests.cpp (line 233) <https://reviews.apache.org/r/43824/#comment184730> `getAllocations` is too general a name for what this function actually does. More like `handleTwoAllocationsAndRecoverResources`. Any method that starts with `get` should return something, preferably whatever comes after `get` (i.e. `Allocations`). If you parameterize the number of iterations of the loop, add a boolean/enum for whether to recover the resources, and return `frameworkAllocations`, then you could call it `getAllocations`, and use it for the 3-iteration loop at the end of the test. That said, only two reuses may not be worth pulling it out (although 3 may be). I give you 3 options (your choice): 1. Rename the method appropriately (may require code changes so it's not as ugly as `handleTwoAllocationsAndRecoverResources`). 2. Inline the method back into the test in both places. 3. Generalize the method and reuse it for the 3rd instance. - Adam B On March 8, 2016, 4:10 a.m., Yongqiao Wang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/43824/ > ----------------------------------------------------------- > > (Updated March 8, 2016, 4:10 a.m.) > > > Review request for mesos, Adam B and Alexander Rukletsov. > > > Bugs: MESOS-4200 > https://issues.apache.org/jira/browse/MESOS-4200 > > > Repository: mesos > > > Description > ------- > > Addressed comments of 41672. > > > Diffs > ----- > > src/tests/hierarchical_allocator_tests.cpp > 3e4ad31925e1b815a74d67fa3962d23fa5bc89d1 > > Diff: https://reviews.apache.org/r/43824/diff/ > > > Testing > ------- > > make && make check successfully. > > > Thanks, > > Yongqiao Wang > >
