> On March 4, 2016, 11:59 a.m., Bernd Mathiske wrote:
> > "{}" is short, but cryptic. It is unclear what kind of entity is being
> > passed here. "EMPTY" was not any better. "hashmap<SlaveID, Resources>()" at
> > least revealed the type which hinted a little at the presumable purpose. A
> > better identifier than "EMPTY" would beat all of the above. Maybe
> > "NO_USED_RESOURCES"?
> >
> > Could the parameter of the methods in question be supplied with a default
> > value? Then no corresponding argument at all would appear in these tests.
I'm a bit reluctant to adding a default value, because these are public
functions from an `Allocator` interface. In previous versions of the patch I
introduced verbose constants like `NO_USED_RESOURCES` and `NO_ALLOCATIONS` but
we opted for more cryptic but shorter `{}`. Functions `addSlave()` and
`addFramework()` are crucial to allocator tests, hence motivating people to
look at their signatures is not a bad idea : ).
- Alexander
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44334/#review122058
-----------------------------------------------------------
On March 3, 2016, 1 p.m., Alexander Rukletsov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44334/
> -----------------------------------------------------------
>
> (Updated March 3, 2016, 1 p.m.)
>
>
> Review request for mesos, Bernd Mathiske and Ben Mahler.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/tests/hierarchical_allocator_tests.cpp
> 3e4ad31925e1b815a74d67fa3962d23fa5bc89d1
>
> Diff: https://reviews.apache.org/r/44334/diff/
>
>
> Testing
> -------
>
> Tested as a chain in https://reviews.apache.org/r/44336/
>
>
> Thanks,
>
> Alexander Rukletsov
>
>
