Re: Review Request 34662: Modularized ResourceEstimator and added test for RE module

2015-06-02 Thread Niklas Nielsen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34662/#review86263 --- Ship it! Modulo Jie's comments - Niklas Nielsen On June 1,

Re: Review Request 34662: Modularized ResourceEstimator and added test for RE module

2015-06-02 Thread Bartek Plotka
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34662/ --- (Updated June 2, 2015, 8:58 p.m.) Review request for mesos, Jie Yu, Niklas

Re: Review Request 34662: Modularized ResourceEstimator and added test for RE module

2015-06-02 Thread Bartek Plotka
On June 2, 2015, 5:07 p.m., Jie Yu wrote: src/tests/module.hpp, lines 76-79 https://reviews.apache.org/r/34662/diff/14/?file=976049#file976049line76 Do we need this? Nope, we deleted RE moduled test so in fact we don't - sorry. - Bartek

Re: Review Request 34662: Modularized ResourceEstimator and added test for RE module

2015-06-02 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34662/#review86247 --- Ship it! src/examples/test_resource_estimator_module.cpp

Re: Review Request 34662: Modularized ResourceEstimator and added test for RE module

2015-06-01 Thread Bartek Plotka
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34662/ --- (Updated June 1, 2015, 6:12 a.m.) Review request for mesos, Jie Yu, Niklas

Re: Review Request 34662: Modularized ResourceEstimator and added test for RE module

2015-06-01 Thread Bartek Plotka
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34662/ --- (Updated June 1, 2015, 6:13 a.m.) Review request for mesos, Jie Yu, Niklas

Re: Review Request 34662: Modularized ResourceEstimator and added test for RE module

2015-06-01 Thread Niklas Nielsen
On June 1, 2015, 1:55 p.m., Jie Yu wrote: Could you please split out the module definition for ResourceEstimator and make this patch not dependent on r34816? I need this patch to build Fixed Resource Estimator. Can't we just get this small chain in a good shape and land it? :) -

Re: Review Request 34662: Modularized ResourceEstimator and added test for RE module

2015-06-01 Thread Jie Yu
On June 1, 2015, 8:55 p.m., Jie Yu wrote: src/tests/module.hpp, lines 76-84 https://reviews.apache.org/r/34662/diff/11/?file=975514#file975514line76 Not your fault. But I found this very unintuitive. It takes me quite a while to figure out why we need this. It would

Re: Review Request 34662: Modularized ResourceEstimator and added test for RE module

2015-06-01 Thread Niklas Nielsen
On June 1, 2015, 1:55 p.m., Jie Yu wrote: src/tests/module.hpp, lines 76-84 https://reviews.apache.org/r/34662/diff/11/?file=975514#file975514line76 Not your fault. But I found this very unintuitive. It takes me quite a while to figure out why we need this. It would

Re: Review Request 34662: Modularized ResourceEstimator and added test for RE module

2015-06-01 Thread Bartek Plotka
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34662/ --- (Updated June 2, 2015, 12:19 a.m.) Review request for mesos, Jie Yu, Niklas

Re: Review Request 34662: Modularized ResourceEstimator and added test for RE module

2015-06-01 Thread Bartek Plotka
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34662/ --- (Updated June 2, 2015, 12:40 a.m.) Review request for mesos, Jie Yu, Niklas

Re: Review Request 34662: Modularized ResourceEstimator and added test for RE module

2015-05-28 Thread Niklas Nielsen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34662/#review85606 --- Ship it! Ship It! - Niklas Nielsen On May 27, 2015, 6:20 p.m.,

Re: Review Request 34662: Modularized ResourceEstimator and added test for RE module

2015-05-28 Thread Niklas Nielsen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34662/#review85613 --- src/examples/test_resource_estimator_module.cpp

Re: Review Request 34662: Modularized ResourceEstimator and added test for RE module

2015-05-28 Thread Bartek Plotka
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34662/ --- (Updated May 28, 2015, 9:23 p.m.) Review request for mesos, Jie Yu, Niklas

Re: Review Request 34662: Modularized ResourceEstimator and added test for RE module

2015-05-28 Thread Niklas Nielsen
On May 27, 2015, 5:24 p.m., Vinod Kone wrote: src/examples/test_resource_estimator_module.cpp, lines 40-44 https://reviews.apache.org/r/34662/diff/6/?file=973211#file973211line40 I'm confused. 'parameters' are unused and the passed 'flags' are empty? Niklas Nielsen wrote:

Re: Review Request 34662: Modularized ResourceEstimator and added test for RE module

2015-05-28 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34662/#review85533 --- Patch looks great! Reviews applied: [34662] All tests passed. -

Re: Review Request 34662: Modularized ResourceEstimator and added test for RE module

2015-05-27 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34662/#review85336 --- Bad patch! Reviews applied: [34662] Failed command: make -j3

Re: Review Request 34662: Modularized ResourceEstimator and added test for RE module

2015-05-27 Thread Bartek Plotka
On May 27, 2015, 9 a.m., Mesos ReviewBot wrote: Bad patch! Reviews applied: [34662] Failed command: make -j3 distcheck Error: make dist-gzip am__post_remove_distdir='@:' make[1]: Entering directory `/home/jenkins/jenkins-slave/workspace/mesos-reviewbot' if test -d

Re: Review Request 34662: Modularized ResourceEstimator and added test for RE module

2015-05-27 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34662/#review85420 --- Patch looks great! Reviews applied: [34662] All tests passed. -

Re: Review Request 34662: Modularized ResourceEstimator and added test for RE module

2015-05-27 Thread Bartek Plotka
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34662/ --- (Updated May 27, 2015, 4:34 p.m.) Review request for mesos, Jie Yu, Niklas

Re: Review Request 34662: Modularized ResourceEstimator and added test for RE module

2015-05-27 Thread Bartłomiej Płotka
Thx, found the issue (: 2015-05-27 8:39 GMT-07:00 Niklas Nielsen nik...@mesosphere.io: +1 to Vinod's suggestion. The buildbot does a dist build, so also make sure new public headers are in src/Makefile.am On 27 May 2015 at 08:32, Vinod Kone vi...@twitter.com.invalid wrote: You likely

Re: Review Request 34662: Modularized ResourceEstimator and added test for RE module

2015-05-27 Thread Bartek Plotka
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34662/ --- (Updated May 28, 2015, 12:04 a.m.) Review request for mesos, Jie Yu, Niklas

Re: Review Request 34662: Modularized ResourceEstimator and added test for RE module

2015-05-27 Thread Bartek Plotka
On May 28, 2015, 12:24 a.m., Vinod Kone wrote: src/examples/test_resource_estimator_module.cpp, lines 40-44 https://reviews.apache.org/r/34662/diff/6/?file=973211#file973211line40 I'm confused. 'parameters' are unused and the passed 'flags' are empty? Niklas Nielsen wrote:

Re: Review Request 34662: Modularized ResourceEstimator and added test for RE module

2015-05-27 Thread Niklas Nielsen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34662/#review85413 --- Do you want to wire up module loading in

Re: Review Request 34662: Modularized ResourceEstimator and added test for RE module

2015-05-26 Thread Bartek Plotka
On May 26, 2015, 8:13 p.m., Niklas Nielsen wrote: src/tests/resource_estimator.hpp, line 40 https://reviews.apache.org/r/34662/diff/2/?file=971547#file971547line40 Put { on a newline Why have create take flags? We tried to make it only take a string. Yeah, but it was needed

Re: Review Request 34662: Modularized ResourceEstimator and added test for RE module

2015-05-26 Thread Niklas Nielsen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34662/#review85304 --- Small nits - we are close :) src/Makefile.am

Re: Review Request 34662: Modularized ResourceEstimator and added test for RE module

2015-05-26 Thread Niklas Nielsen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34662/#review85225 --- src/Makefile.am https://reviews.apache.org/r/34662/#comment136757

Re: Review Request 34662: Modularized ResourceEstimator and added test for RE module

2015-05-25 Thread Bartek Plotka
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34662/ --- (Updated May 26, 2015, 5:29 a.m.) Review request for mesos, Jie Yu, Niklas