Re: Review Request 31265: Provided a factory for allocator in tests.

2015-04-21 Thread Niklas Nielsen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31265/#review81038 --- Ship it! Ship It! - Niklas Nielsen On April 21, 2015, 11:45

Re: Review Request 31265: Provided a factory for allocator in tests.

2015-04-21 Thread Alexander Rukletsov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31265/ --- (Updated April 21, 2015, 6:03 p.m.) Review request for mesos, Kapil Arya,

Re: Review Request 31265: Provided a factory for allocator in tests.

2015-04-21 Thread Alexander Rukletsov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31265/ --- (Updated April 21, 2015, 6:45 p.m.) Review request for mesos, Kapil Arya,

Re: Review Request 31265: Provided a factory for allocator in tests.

2015-04-20 Thread Alexander Rukletsov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31265/ --- (Updated April 20, 2015, 1:50 p.m.) Review request for mesos, Kapil Arya,

Re: Review Request 31265: Provided a factory for allocator in tests.

2015-04-19 Thread Alexander Rukletsov
On April 17, 2015, 1:31 p.m., Michael Park wrote: src/local/local.cpp, lines 141-145 https://reviews.apache.org/r/31265/diff/8/?file=929832#file929832line141 What's the point of setting `_allocator` here? It looks like it only gets passed to the `new Master(...)` call.

Re: Review Request 31265: Provided a factory for allocator in tests.

2015-04-19 Thread Alexander Rukletsov
On April 17, 2015, 1:31 p.m., Michael Park wrote: src/tests/mesos.hpp, lines 866-874 https://reviews.apache.org/r/31265/diff/8/?file=929838#file929838line866 An idea to avoid this: we could promote the `createAllocator` currently in `MasterAllocatorTest` to this file. Then we

Re: Review Request 31265: Provided a factory for allocator in tests.

2015-04-17 Thread Michael Park
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31265/#review80445 --- src/local/local.cpp

Re: Review Request 31265: Provided a factory for allocator in tests.

2015-04-17 Thread Michael Park
On April 17, 2015, 1:31 p.m., Michael Park wrote: src/tests/mesos.hpp, lines 866-874 https://reviews.apache.org/r/31265/diff/8/?file=929838#file929838line866 An idea to avoid this: we could promote the `createAllocator` currently in `MasterAllocatorTest` to this file. Then we

Re: Review Request 31265: Provided a factory for allocator in tests.

2015-04-17 Thread Michael Park
On April 17, 2015, 1:31 p.m., Michael Park wrote: src/tests/hierarchical_allocator_tests.cpp, line 77 https://reviews.apache.org/r/31265/diff/8/?file=929836#file929836line77 I see this is the place where you wanted to use `ASSERT_SOME` but couldn't due to the [gtest

Re: Review Request 31265: Provided a factory for allocator in tests.

2015-04-15 Thread Alexander Rukletsov
On April 2, 2015, 7:59 p.m., Vinod Kone wrote: src/master/allocator/mesos/allocator.hpp, line 47 https://reviews.apache.org/r/31265/diff/6/?file=912110#file912110line47 Do you still want the constructor public? Alexander Rukletsov wrote: I think there is no reason to hide

Re: Review Request 31265: Provided a factory for allocator in tests.

2015-04-15 Thread Alexander Rukletsov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31265/ --- (Updated April 15, 2015, 2:19 p.m.) Review request for mesos, Kapil Arya,

Re: Review Request 31265: Provided a factory for allocator in tests.

2015-04-07 Thread Alexander Rukletsov
On April 2, 2015, 7:59 p.m., Vinod Kone wrote: src/master/allocator/mesos/allocator.hpp, line 47 https://reviews.apache.org/r/31265/diff/6/?file=912110#file912110line47 Do you still want the constructor public? I think there is no reason to hide the c-tor. I can imagine somebody

Re: Review Request 31265: Provided a factory for allocator in tests.

2015-04-07 Thread Alexander Rukletsov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31265/ --- (Updated April 7, 2015, 12:49 p.m.) Review request for mesos, Kapil Arya,

Re: Review Request 31265: Provided a factory for allocator in tests.

2015-04-02 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31265/#review78711 --- src/master/allocator/mesos/allocator.hpp

Re: Review Request 31265: Provided a factory for allocator in tests.

2015-04-01 Thread Alexander Rukletsov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31265/ --- (Updated April 1, 2015, 2:28 p.m.) Review request for mesos, Kapil Arya,

Re: Review Request 31265: Provided a factory for allocator in tests.

2015-03-27 Thread Alexander Rukletsov
On March 24, 2015, 2:41 p.m., Michael Park wrote: src/tests/master_allocator_tests.cpp, line 97 https://reviews.apache.org/r/31265/diff/4/?file=903029#file903029line97 1. Do we need this to be `virtual` right now? 2. `s/createAllocatorInstance/CreateAllocator/` 3. Can we

Re: Review Request 31265: Provided a factory for allocator in tests.

2015-03-27 Thread Michael Park
On March 24, 2015, 2:41 p.m., Michael Park wrote: src/tests/master_allocator_tests.cpp, line 97 https://reviews.apache.org/r/31265/diff/4/?file=903029#file903029line97 1. Do we need this to be `virtual` right now? 2. `s/createAllocatorInstance/CreateAllocator/` 3. Can we

Re: Review Request 31265: Provided a factory for allocator in tests.

2015-03-27 Thread Alexander Rukletsov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31265/ --- (Updated March 27, 2015, 4:27 p.m.) Review request for mesos, Kapil Arya,

Re: Review Request 31265: Provided a factory for allocator in tests.

2015-03-24 Thread Michael Park
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31265/#review77570 --- src/tests/master_allocator_tests.cpp

Re: Review Request 31265: Provided a factory for allocator in tests.

2015-03-23 Thread Alexander Rukletsov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31265/ --- (Updated March 23, 2015, 2:11 p.m.) Review request for mesos, Kapil Arya,

Re: Review Request 31265: Provided a factory for allocator in tests.

2015-03-16 Thread Niklas Nielsen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31265/#review76679 --- For consistency, I would still root for Allocator::create()

Re: Review Request 31265: Provided a factory for allocator in tests.

2015-03-13 Thread Alexander Rukletsov
On March 10, 2015, 9:11 p.m., Niklas Nielsen wrote: How about we try to make this look like the Containerizer factory-like method (Containerizer::create)? That would be a bit more consistent I think `Containerizer` is not a good example here. I would rather point to `Authenticator` and /

Re: Review Request 31265: Provided a factory for allocator in tests.

2015-03-13 Thread Alexander Rukletsov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31265/ --- (Updated March 13, 2015, 9:50 p.m.) Review request for mesos, Kapil Arya,

Re: Review Request 31265: Provided a factory for allocator in tests.

2015-03-10 Thread Niklas Nielsen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31265/#review75978 --- How about we try to make this look like the Containerizer

Re: Review Request 31265: Provided a factory for allocator in tests.

2015-03-05 Thread Alexander Rukletsov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31265/ --- (Updated March 5, 2015, 9:07 p.m.) Review request for mesos, Kapil Arya,