Re: Review Request 31776: Moved allocator to public headers.

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

Re: Review Request 31776: Moved allocator to public headers.

2015-04-20 Thread Alexander Rukletsov
On April 2, 2015, 6:23 p.m., Vinod Kone wrote: include/mesos/master/allocator.proto, lines 19-23 https://reviews.apache.org/r/31776/diff/6/?file=912077#file912077line19 Since allocator is within the same Unix process as Master, what is the compatibility issue here? Alexander

Re: Review Request 31776: Moved allocator to public headers.

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

Re: Review Request 31776: Moved allocator to public headers.

2015-04-07 Thread Alexander Rukletsov
On April 2, 2015, 6:23 p.m., Vinod Kone wrote: include/mesos/master/allocator.proto, lines 19-23 https://reviews.apache.org/r/31776/diff/6/?file=912077#file912077line19 Since allocator is within the same Unix process as Master, what is the compatibility issue here? The comment

Re: Review Request 31776: Moved allocator to public headers.

2015-04-07 Thread Alexander Rukletsov
On April 2, 2015, 6:23 p.m., Vinod Kone wrote: src/Makefile.am, line 711 https://reviews.apache.org/r/31776/diff/6/?file=912078#file912078line711 not yours, but can you kill the trailing white space here? Sure. - Alexander

Re: Review Request 31776: Moved allocator to public headers.

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

Re: Review Request 31776: Moved allocator to public headers.

2015-04-02 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31776/#review78690 --- Ship it! include/mesos/master/allocator.proto

Re: Review Request 31776: Moved allocator to public headers.

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

Re: Review Request 31776: Moved allocator to public headers.

2015-03-27 Thread Alexander Rukletsov
On March 23, 2015, 10:27 p.m., Michael Park wrote: include/mesos/master/allocator.proto, lines 20-21 https://reviews.apache.org/r/31776/diff/4/?file=902995#file902995line20 `s/breaking the backward compatibility/breaking backwards compatibility/` Also this is

Re: Review Request 31776: Moved allocator to public headers.

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

Re: Review Request 31776: Moved allocator to public headers.

2015-03-27 Thread Michael Park
On March 23, 2015, 10:27 p.m., Michael Park wrote: include/mesos/master/allocator.proto, lines 20-21 https://reviews.apache.org/r/31776/diff/4/?file=902995#file902995line20 `s/breaking the backward compatibility/breaking backwards compatibility/` Also this is

Re: Review Request 31776: Moved allocator to public headers.

2015-03-26 Thread Alexander Rojas
On March 23, 2015, 11:27 p.m., Michael Park wrote: src/local/local.hpp, line 28 https://reviews.apache.org/r/31776/diff/4/?file=902997#file902997line28 This comment looks useless. Can we just get rid of it? +1 On March 23, 2015, 11:27 p.m., Michael Park wrote:

Re: Review Request 31776: Moved allocator to public headers.

2015-03-24 Thread Kapil Arya
On March 23, 2015, 6:27 p.m., Michael Park wrote: src/Makefile.am, lines 234-239 https://reviews.apache.org/r/31776/diff/4/?file=902996#file902996line234 In line with the above comment, it seems like `$(ALLOCATOR_PROTO)` should live in `include/mesos/allocator/...` rather than

Re: Review Request 31776: Moved allocator to public headers.

2015-03-24 Thread Michael Park
On March 23, 2015, 10:27 p.m., Michael Park wrote: src/Makefile.am, lines 234-239 https://reviews.apache.org/r/31776/diff/4/?file=902996#file902996line234 In line with the above comment, it seems like `$(ALLOCATOR_PROTO)` should live in `include/mesos/allocator/...` rather than

Re: Review Request 31776: Moved allocator to public headers.

2015-03-24 Thread Michael Park
On March 23, 2015, 10:27 p.m., Michael Park wrote: src/Makefile.am, lines 170-171 https://reviews.apache.org/r/31776/diff/4/?file=902996#file902996line170 Seems to me like these should be `allocator/allocator.pb.cc`, and `../include/mesos/allocator/allocator.pb.h`. Could you

Re: Review Request 31776: Moved allocator to public headers.

2015-03-23 Thread Kapil Arya
On March 17, 2015, 4:58 p.m., Kapil Arya wrote: src/master/allocator/allocator.hpp, line 39 https://reviews.apache.org/r/31776/diff/3/?file=894463#file894463line39 Why do we have a separate allocator namespace? Wouldn't it be better to just put the Allocator class in

Re: Review Request 31776: Moved allocator to public headers.

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

Re: Review Request 31776: Moved allocator to public headers.

2015-03-23 Thread Michael Park
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31776/#review77477 --- include/mesos/master/allocator.proto

Re: Review Request 31776: Moved allocator to public headers.

2015-03-18 Thread Alexander Rukletsov
On March 17, 2015, 8:58 p.m., Kapil Arya wrote: src/master/allocator/allocator.hpp, line 39 https://reviews.apache.org/r/31776/diff/3/?file=894463#file894463line39 Why do we have a separate allocator namespace? Wouldn't it be better to just put the Allocator class in

Re: Review Request 31776: Moved allocator to public headers.

2015-03-17 Thread Kapil Arya
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31776/#review76805 --- Ship it! The review looks good to me. However I have added one

Re: Review Request 31776: Moved allocator to public headers.

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

Re: Review Request 31776: Moved allocator to public headers.

2015-03-12 Thread Alexander Rukletsov
On March 10, 2015, 9:09 p.m., Niklas Nielsen wrote: src/local/local.hpp, lines 26-46 https://reviews.apache.org/r/31776/diff/2/?file=886202#file886202line26 Can we include the allocator and master header instead? Let's include allocator header and leave fwd decl master to keep

Re: Review Request 31776: Moved allocator to public headers.

2015-03-10 Thread Niklas Nielsen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31776/#review75894 --- include/mesos/master/allocator.proto

Review Request 31776: Moved allocator to public headers.

2015-03-05 Thread Alexander Rukletsov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31776/ --- Review request for mesos, Kapil Arya, Michael Park, and Niklas Nielsen. Bugs:

Re: Review Request 31776: Moved allocator to public headers.

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