Re: Review Request 38342: Add JSON::protobuf for google::protobuf::RepeatedPtrField (stout part)

2015-09-25 Thread Klaus Ma
> On Sept. 25, 2015, 1:35 p.m., Alexander Rukletsov wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp, line 609 > > > > > > 1. You can also use `inline`, that's what we usually do. > > 2.

Re: Review Request 37993: Add explanatory comments for Allocator interface

2015-09-25 Thread Guangya Liu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37993/ --- (Updated 九月 25, 2015, 4:21 p.m.) Review request for mesos, Adam B, Alexander

Re: Review Request 38747: Adding digest utilities

2015-09-25 Thread Jojy Varghese
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38747/ --- (Updated Sept. 25, 2015, 3:23 p.m.) Review request for mesos, Gilbert Song and

Re: Review Request 38542: CMake: Use version info from `Versions.cmake` instead of magic strings.

2015-09-25 Thread haosdent huang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38542/#review100585 --- Ship it! Ship It! - haosdent huang On Sept. 21, 2015, 3:23

Re: Review Request 38549: [1/2]CMake: Add libevent version, configure Windows to use as default.

2015-09-25 Thread haosdent huang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38549/#review100586 --- Ship it! Ship It! - haosdent huang On Sept. 21, 2015, 6:31

Re: Review Request 38550: [2/2]CMake: Integrate libevent into Windows builds.

2015-09-25 Thread haosdent huang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38550/#review100587 --- Ship it! Ship It! - haosdent huang On Sept. 21, 2015, 10:43

Re: Review Request 38541: CMake: Add `Versions.cmake` as an analog to `versions.am`.

2015-09-25 Thread haosdent huang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38541/#review100584 --- Ship it! Ship It! - haosdent huang On Sept. 21, 2015, 6:25

Re: Review Request 37197: Docker image store.

2015-09-25 Thread Timothy Chen
> On Sept. 3, 2015, 6:16 p.m., Jojy Varghese wrote: > > src/slave/containerizer/provisioners/docker/store.hpp, line 49 > > > > > > Is this intended to be an abstract class? If so, why this factory > > method? It's

Re: Review Request 38747: Adding digest utilities

2015-09-25 Thread Jojy Varghese
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38747/ --- (Updated Sept. 25, 2015, 4:45 p.m.) Review request for mesos, Gilbert Song and

Re: Review Request 38541: CMake: Add `Versions.cmake` as an analog to `versions.am`.

2015-09-25 Thread Artem Harutyunyan
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38541/#review100600 --- CMakeLists.txt (line 41)

Re: Review Request 38549: [1/2]CMake: Add libevent version, configure Windows to use as default.

2015-09-25 Thread Artem Harutyunyan
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38549/#review100607 --- LGTM. - Artem Harutyunyan On Sept. 21, 2015, 11:31 a.m., Alex

Re: Review Request 38754: CMake: Add build/configure/install logic for Zookeeper.

2015-09-25 Thread Joseph Wu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38754/#review100614 --- Ship it! 3rdparty/CMakeLists.txt (lines 25 - 26)

Re: Review Request 38752: CMake: Transition Mesos to use new third-party build scripts.

2015-09-25 Thread Artem Harutyunyan
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38752/#review100616 --- LGTM. - Artem Harutyunyan On Sept. 24, 2015, 11:18 p.m., Alex

Re: Review Request 38667: Added a test for os::realpath().

2015-09-25 Thread Joris Van Remoortere
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38667/#review100626 --- Can we simplify this by using: `os::mktemp` `os::stat::inode` to

Re: Review Request 37540: Add perf event API

2015-09-25 Thread Cong Wang
> On Sept. 22, 2015, 12:37 a.m., Jie Yu wrote: > > src/linux/perf.cpp, lines 516-529 > > > > > > Should we use `__sync_synchronize` instead? Hmm, __sync_synchronize is documented as a full memory barrier, while we

Re: Review Request 38770: routing: fixed a file descriptor leak.

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

Re: Review Request 38577: Added synchronous validation for Call in Agent

2015-09-25 Thread Anand Mazumdar
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38577/#review100682 --- src/slave/validation.cpp (line 52)

Re: Review Request 38774: state: fix file descriptor leak

2015-09-25 Thread Chi Zhang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38774/ --- (Updated Sept. 25, 2015, 9 p.m.) Review request for mesos. Bugs: mesos-3519

Re: Review Request 38774: state: fix file descriptor leak

2015-09-25 Thread Cong Wang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38774/#review100688 --- src/slave/state.cpp (line 656)

Re: Review Request 38743: files: fix potential fd leaks under error conditions.

2015-09-25 Thread Chi Zhang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38743/ --- (Updated Sept. 25, 2015, 8:34 p.m.) Review request for mesos. Bugs:

Re: Review Request 38747: Adding digest utilities

2015-09-25 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38747/#review100686 --- Patch looks great! Reviews applied: [38443, 38579, 38580, 38747]

Re: Review Request 38756: CMake: Add support for compiling the agent with CMake.

2015-09-25 Thread Artem Harutyunyan
> On Sept. 25, 2015, 2:36 p.m., Artem Harutyunyan wrote: > > LGTM. - Artem --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38756/#review100641

Re: Review Request 38756: CMake: Add support for compiling the agent with CMake.

2015-09-25 Thread Artem Harutyunyan
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38756/#review100641 --- src/CMakeLists.txt (line 1)

Re: Review Request 38752: CMake: Transition Mesos to use new third-party build scripts.

2015-09-25 Thread Joris Van Remoortere
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38752/#review100676 --- You'll need to split this up as it's touching 2 different

Re: Review Request 38770: routing: fixed a file descriptor leak.

2015-09-25 Thread Chi Zhang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38770/ --- (Updated Sept. 25, 2015, 8:33 p.m.) Review request for mesos, Ben Mahler, Jie

Re: Review Request 38747: Adding digest utilities

2015-09-25 Thread Jojy Varghese
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38747/ --- (Updated Sept. 25, 2015, 8:33 p.m.) Review request for mesos, Gilbert Song and

Review Request 38774: state: fix file descriptor leak

2015-09-25 Thread Chi Zhang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38774/ --- Review request for mesos. Bugs: mesos-3519

Re: Review Request 38774: state: fix file descriptor leak

2015-09-25 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38774/#review100690 --- Bad review! Reviews applied: [] Error: No reviewers specified.

Re: Review Request 38550: [2/2]CMake: Integrate libevent into Windows builds.

2015-09-25 Thread Artem Harutyunyan
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38550/#review100608 --- 3rdparty/libprocess/3rdparty/CMakeLists.txt (line 61)

Re: Review Request 38551: [1/2]CMake: Add version info for APR we need to build Windows.

2015-09-25 Thread Artem Harutyunyan
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38551/#review100610 --- LGTM. - Artem Harutyunyan On Sept. 21, 2015, 11:30 a.m., Alex

Re: Review Request 37993: Add explanatory comments for Allocator interface

2015-09-25 Thread Guangya Liu
> On 九月 24, 2015, 4:35 p.m., Alexander Rukletsov wrote: > > Looking almost perfect! Really appreciate that you diligently address all > > the comments I throw into you and don't give up. > > > > While you're touching this file, I would say it makes sense to clean up > > everything we can in

Re: Review Request 38756: CMake: Add support for compiling the agent with CMake.

2015-09-25 Thread Joseph Wu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38756/#review100625 --- src/CMakeLists.txt (lines 34 - 38)

Re: Review Request 38539: [VIA HAOSDENT] [1/2]Add CMake macro VsBuildCommand in libprocess.

2015-09-25 Thread Alex Clemmer
> On Sept. 25, 2015, 4:46 p.m., Artem Harutyunyan wrote: > > 3rdparty/libprocess/3rdparty/CMakeLists.txt, line 242 > > > > > > This surpasses 80 character limit, is there a way to fix it here and > > elsewhere in

Re: Review Request 38755: CMake: Move compiler configuration logic to its own file.

2015-09-25 Thread Artem Harutyunyan
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38755/#review100637 --- cmake/CompilationConfigure.cmake (line 84)

Re: Review Request 38577: Added synchronous validation for Call in Agent

2015-09-25 Thread Isabel Jimenez
> On Sept. 25, 2015, 6:52 p.m., Vinod Kone wrote: > > src/slave/validation.hpp, line 36 > > > > > > Can you please add tests in this patch? Adding tests in a separate dependant patch. - Isabel

Re: Review Request 38770: routing: fixed a file descriptor leak.

2015-09-25 Thread Chi Zhang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38770/ --- (Updated Sept. 25, 2015, 7:07 p.m.) Review request for mesos, Ben Mahler, Jie

Re: Review Request 38770: routing: fixed a file descriptor leak.

2015-09-25 Thread Cong Wang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38770/#review100661 --- Ship it! Or you can do unconditionally os::close() after that

Re: Review Request 38756: CMake: Add support for compiling the agent with CMake.

2015-09-25 Thread Alex Clemmer
> On Sept. 25, 2015, 5:39 p.m., Joseph Wu wrote: > > src/CMakeLists.txt, lines 34-38 > > > > > > This seems macro-able. > > > > A macro which would let you do something like: > > ``` > >

Re: Review Request 38756: CMake: Add support for compiling the agent with CMake.

2015-09-25 Thread Joseph Wu
> On Sept. 25, 2015, 10:39 a.m., Joseph Wu wrote: > > src/CMakeLists.txt, lines 34-38 > > > > > > This seems macro-able. > > > > A macro which would let you do something like: > > ``` > >

Re: Review Request 38577: Added synchronous validation for Call in Agent

2015-09-25 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38577/#review100653 --- src/Makefile.am (line 501)

Re: Review Request 37993: Add explanatory comments for Allocator interface

2015-09-25 Thread Joseph Wu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37993/#review100628 --- Per AlexR, I'll review as a native speaker. General note: There

Review Request 38770: routing: fixed a file descriptor leak.

2015-09-25 Thread Chi Zhang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38770/ --- Review request for mesos, Ben Mahler, Jie Yu, and Cong Wang. Repository: mesos

Re: Review Request 38770: routing: fixed a file descriptor leak.

2015-09-25 Thread Ben Mahler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38770/#review100670 --- Ship it! Thanks! Would you mind tracking these under a ticket to

Re: Review Request 38468: docs: Added discussion of finding a shepherd.

2015-09-25 Thread Joseph Wu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38468/#review100671 --- Ship it! Ship It! - Joseph Wu On Sept. 24, 2015, 2:29 p.m.,

Re: Review Request 38756: CMake: Add support for compiling the agent with CMake.

2015-09-25 Thread Joseph Wu
> On Sept. 25, 2015, 12:06 a.m., Mesos ReviewBot wrote: > > Bad patch! > > > > Reviews applied: [38456, 38457, 38529, 38530, 38531, 38538, 38539, 38540, > > 38541, 38542, 38549, 38550, 38551, 38552, 38751, 38752] > > > > Failed command: ./support/apply-review.sh -n -r 38752 > > > > Error: >

Re: Review Request 38531: CMake: Update CMake config to build Mesos against picojson v1.3.0.

2015-09-25 Thread Artem Harutyunyan
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38531/#review100593 --- LGTM. - Artem Harutyunyan On Sept. 19, 2015, 7:07 p.m., Alex

Re: Review Request 38530: CMake: Add preprocessor definitions required to build picojson 1.3.0.

2015-09-25 Thread Artem Harutyunyan
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38530/#review100592 --- LGTM. - Artem Harutyunyan On Sept. 21, 2015, 11:06 a.m., Alex

Re: Review Request 38539: [VIA HAOSDENT] [1/2]Add CMake macro VsBuildCommand in libprocess.

2015-09-25 Thread Artem Harutyunyan
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38539/#review100598 --- LGTM (- the lines that are too long). - Artem Harutyunyan On

Re: Review Request 38540: [VIA HAOSDENT] [2/2]Generate make batch file to build project in windows.

2015-09-25 Thread Artem Harutyunyan
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38540/#review100599 --- LGTM. - Artem Harutyunyan On Sept. 20, 2015, 7:41 p.m., Alex

Re: Review Request 38542: CMake: Use version info from `Versions.cmake` instead of magic strings.

2015-09-25 Thread Artem Harutyunyan
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38542/#review100604 --- LGTM. - Artem Harutyunyan On Sept. 20, 2015, 8:23 p.m., Alex

Re: Review Request 38595: Removed implicit logging from Socket::shutdown in favor of returning a Try.

2015-09-25 Thread Joris Van Remoortere
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38595/#review100630 --- Ship it! Could you ping me to discuss this question "should this

Re: Review Request 38705: Added support for applying a review chain (apply-reviews.py).

2015-09-25 Thread Vinod Kone
> On Sept. 25, 2015, 4:35 p.m., Joseph Wu wrote: > > One more thing: when the patch fails to apply, I get this message: > > > > ``` > > Traceback (most recent call last): > > File "support/apply-reviews.py", line 86, in > > apply_review(r, dry_run) > > File "support/apply-reviews.py",

Re: Review Request 38747: Adding digest utilities

2015-09-25 Thread Jojy Varghese
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38747/ --- (Updated Sept. 25, 2015, 6:33 p.m.) Review request for mesos, Gilbert Song and

Re: Review Request 38705: Added support for applying a review chain (apply-reviews.py).

2015-09-25 Thread Joseph Wu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38705/#review100588 --- One more thing: when the patch fails to apply, I get this message:

Re: Review Request 38529: CMake: Only compile proc_tests.cpp for Linux platforms.

2015-09-25 Thread Artem Harutyunyan
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38529/#review100591 --- LGTM. - Artem Harutyunyan On Sept. 19, 2015, 7:07 p.m., Alex

Re: Review Request 37197: Docker image store.

2015-09-25 Thread Timothy Chen
> On Aug. 31, 2015, 8:09 p.m., Timothy Chen wrote: > > src/slave/containerizer/provisioners/docker/store.cpp, line 192 > > > > > > I think path::join just returns a string This is removed later on. - Timothy

Re: Review Request 38747: Adding digest utilities

2015-09-25 Thread Jojy Varghese
> On Sept. 25, 2015, 6:24 a.m., Alex Clemmer wrote: > > Hey, it doesn't look like `src/tests/digest_tests.cpp` is being added to > > the `CMakeLists.txt` file in `3rdparty/libprocess/src/tests`. Is that > > correct? If not, could we please add it there as well? > > > > (I have forgotten to

Re: Review Request 38539: [VIA HAOSDENT] [1/2]Add CMake macro VsBuildCommand in libprocess.

2015-09-25 Thread Artem Harutyunyan
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38539/#review100596 --- 3rdparty/libprocess/3rdparty/CMakeLists.txt (line 207)

Re: Review Request 38751: CMake: Pull third-party configuration logic into its own .cmake file.

2015-09-25 Thread Joseph Wu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38751/#review100602 --- Looks good. Won't hurt to mention (in the description) that this

Re: Review Request 38541: CMake: Add `Versions.cmake` as an analog to `versions.am`.

2015-09-25 Thread Artem Harutyunyan
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38541/#review100601 --- LGTM (- the long line). - Artem Harutyunyan On Sept. 21, 2015,

Re: Review Request 38752: CMake: Transition Mesos to use new third-party build scripts.

2015-09-25 Thread Joseph Wu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38752/#review100603 --- Ship it! Ship It! - Joseph Wu On Sept. 24, 2015, 11:18 p.m.,

Re: Review Request 37993: Add explanatory comments for Allocator interface

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

Re: Review Request 38552: [2/2]CMake: Add Windows-specific build targets for APR.

2015-09-25 Thread Artem Harutyunyan
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38552/#review100612 --- LGTM. - Artem Harutyunyan On Sept. 21, 2015, 4:21 a.m., Alex

Re: Review Request 37993: Add explanatory comments for Allocator interface

2015-09-25 Thread Guangya Liu
> On 九月 24, 2015, 4:35 p.m., Alexander Rukletsov wrote: > > include/mesos/master/allocator.hpp, line 54 > > > > > > I think you killed that line in previous versions of the review, why do > > you want to bring it

Re: Review Request 38747: Adding digest utilities

2015-09-25 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38747/#review100619 --- Bad patch! Reviews applied: [38443, 38579] Failed command:

Re: Review Request 37993: Add explanatory comments for Allocator interface

2015-09-25 Thread Guangya Liu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37993/ --- (Updated 九月 25, 2015, 5:22 p.m.) Review request for mesos, Adam B, Alexander

Re: Review Request 38753: CMake: Transition Stout tests to use new third-party build scripts.

2015-09-25 Thread Artem Harutyunyan
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38753/#review100623 --- LGTM. - Artem Harutyunyan On Sept. 24, 2015, 11:19 p.m., Alex

Re: Review Request 38754: CMake: Add build/configure/install logic for Zookeeper.

2015-09-25 Thread Artem Harutyunyan
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38754/#review100631 --- 3rdparty/CMakeLists.txt (line 37)

Re: Review Request 38705: Added support for applying a review chain (apply-reviews.py).

2015-09-25 Thread Joseph Wu
> On Sept. 25, 2015, 9:35 a.m., Joseph Wu wrote: > > One more thing: when the patch fails to apply, I get this message: > > > > ``` > > Traceback (most recent call last): > > File "support/apply-reviews.py", line 86, in > > apply_review(r, dry_run) > > File "support/apply-reviews.py",

Re: Review Request 38338: Enhanced option for Docker cli volume plugin.

2015-09-25 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38338/#review100643 --- Can you add a test for this logic? - Timothy Chen On Sept. 25,

Re: Review Request 38338: Enhanced option for Docker cli volume plugin.

2015-09-25 Thread haosdent huang
> On Sept. 25, 2015, 6:18 p.m., Timothy Chen wrote: > > Can you add a test for this logic? Could, but because volume-driver depends on flocker, my test ideas for this is verify the docker run command is correct. - haosdent --- This is

Re: Review Request 38550: [2/2]CMake: Integrate libevent into Windows builds.

2015-09-25 Thread Alex Clemmer
> On Sept. 25, 2015, 5:03 p.m., Artem Harutyunyan wrote: > > 3rdparty/libprocess/3rdparty/CMakeLists.txt, line 61 > > > > > > 80+ characters? Per conversation with Joris, I think we'll probably keep this as is. -

Re: Review Request 38753: CMake: Transition Stout tests to use new third-party build scripts.

2015-09-25 Thread Alex Clemmer
> On Sept. 25, 2015, 4:58 p.m., Joseph Wu wrote: > > 3rdparty/libprocess/3rdparty/stout/cmake/StoutTestsConfigure.cmake, lines > > 109-126 > > > > > > Just to clarify. You're getting rid of this because of

Re: Review Request 37993: Add explanatory comments for Allocator interface

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

Re: Review Request 38538: [VIA HAOSDENT] CMake: Add `CMAKE_NOOP` to common definitions file.

2015-09-25 Thread Artem Harutyunyan
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38538/#review100595 --- LGTM. - Artem Harutyunyan On Sept. 20, 2015, 7:33 p.m., Alex

Re: Review Request 38753: CMake: Transition Stout tests to use new third-party build scripts.

2015-09-25 Thread Joseph Wu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38753/#review100605 --- Ship it!

Re: Review Request 38754: CMake: Add build/configure/install logic for Zookeeper.

2015-09-25 Thread Alex Clemmer
> On Sept. 25, 2015, 5:12 p.m., Joseph Wu wrote: > > 3rdparty/CMakeLists.txt, lines 25-26 > > > > > > Do you have plans to combine this with the similar lines in > > `3rdparty/libprocess/3rdparty/CMakeLists.txt`? >

Re: Review Request 37993: Add explanatory comments for Allocator interface

2015-09-25 Thread Joseph Wu
> On Sept. 24, 2015, 9:35 a.m., Alexander Rukletsov wrote: > > include/mesos/master/allocator.hpp, lines 60-61 > > > > > > Not yours, but let's wrap all types and variable names in backticks "`" > > Guangya Liu

Re: Review Request 38416: Allow HTTP response codes to be checked with code.

2015-09-25 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38416/#review100633 --- Bad patch! Reviews applied: [38416] Failed command:

Re: Review Request 38539: [VIA HAOSDENT] [1/2]Add CMake macro VsBuildCommand in libprocess.

2015-09-25 Thread Alex Clemmer
> On Sept. 25, 2015, 4:46 p.m., Artem Harutyunyan wrote: > > 3rdparty/libprocess/3rdparty/CMakeLists.txt, line 242 > > > > > > This surpasses 80 character limit, is there a way to fix it here and > > elsewhere in

Re: Review Request 38416: Allow HTTP response codes to be checked with code.

2015-09-25 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38416/ --- (Updated Sept. 25, 2015, 4:50 p.m.) Review request for mesos, Anand Mazumdar,

Re: Review Request 38539: [VIA HAOSDENT] [1/2]Add CMake macro VsBuildCommand in libprocess.

2015-09-25 Thread haosdent huang
> On Sept. 25, 2015, 4:46 p.m., Artem Harutyunyan wrote: > > I don't think so. And Makefile.am also not follow the 80 characters limit. - haosdent --- This is an automatically generated e-mail. To reply, visit:

Re: Review Request 38755: CMake: Move compiler configuration logic to its own file.

2015-09-25 Thread Joseph Wu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38755/#review100618 --- Ship it! cmake/CompilationConfigure.cmake (line 46)

Re: Review Request 38755: CMake: Move compiler configuration logic to its own file.

2015-09-25 Thread Artem Harutyunyan
> On Sept. 25, 2015, 11:05 a.m., Artem Harutyunyan wrote: > > LGTM. - Artem --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38755/#review100637

Re: Review Request 38580: Added docker registry RemotePuller

2015-09-25 Thread Jojy Varghese
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38580/ --- (Updated Sept. 25, 2015, 6:34 p.m.) Review request for mesos and Timothy Chen.

Re: Review Request 38579: Exposing manifest structure outside RegistryClient

2015-09-25 Thread Jojy Varghese
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38579/ --- (Updated Sept. 25, 2015, 6:34 p.m.) Review request for mesos and Timothy Chen.

Re: Review Request 38443: Added layerid information to ManifestResponse

2015-09-25 Thread Jojy Varghese
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38443/ --- (Updated Sept. 25, 2015, 6:33 p.m.) Review request for mesos and Timothy Chen.

Re: Review Request 38619: Added missing header guards for master/validation.hpp

2015-09-25 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38619/#review100651 --- Ship it! Ship It! - Vinod Kone On Sept. 22, 2015, 9:57 a.m.,

Re: Review Request 38539: [VIA HAOSDENT] [1/2]Add CMake macro VsBuildCommand in libprocess.

2015-09-25 Thread Alex Clemmer
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38539/ --- (Updated Sept. 25, 2015, 10:08 p.m.) Review request for mesos, haosdent huang,

Re: Review Request 38751: CMake: Pull third-party configuration logic into its own .cmake file.

2015-09-25 Thread Alex Clemmer
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38751/ --- (Updated Sept. 25, 2015, 10:11 p.m.) Review request for mesos, Artem

Review Request 38777: Avoid closing '-1' file descriptors in Subprocess.

2015-09-25 Thread Ben Mahler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38777/ --- Review request for mesos and Jie Yu. Repository: mesos Description ---

Re: Review Request 38756: CMake: Add support for compiling the agent with CMake.

2015-09-25 Thread Alex Clemmer
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38756/ --- (Updated Sept. 25, 2015, 10:14 p.m.) Review request for mesos, Artem

Re: Review Request 38755: CMake: Move compiler configuration logic to its own file.

2015-09-25 Thread Alex Clemmer
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38755/ --- (Updated Sept. 25, 2015, 10:13 p.m.) Review request for mesos, Artem

Re: Review Request 38755: CMake: Move compiler configuration logic to its own file.

2015-09-25 Thread Alex Clemmer
> On Sept. 25, 2015, 6:05 p.m., Artem Harutyunyan wrote: > > cmake/CompilationConfigure.cmake, line 84 > > > > > > NOLINT? Doesn't complain on my box, I don't think it's necessary. Also, this _does_ run when you

Re: Review Request 38777: Avoid closing '-1' file descriptors in Subprocess.

2015-09-25 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38777/#review100697 --- Ship it! Ship It! - Jie Yu On Sept. 25, 2015, 10:14 p.m., Ben

Re: Review Request 38756: CMake: Add support for compiling the agent with CMake.

2015-09-25 Thread Alex Clemmer
> On Sept. 25, 2015, 7:06 a.m., Mesos ReviewBot wrote: > > Bad patch! > > > > Reviews applied: [38456, 38457, 38529, 38530, 38531, 38538, 38539, 38540, > > 38541, 38542, 38549, 38550, 38551, 38552, 38751, 38752] > > > > Failed command: ./support/apply-review.sh -n -r 38752 > > > > Error: > >

Re: Review Request 38776: Removed unnecessary close calls on ifstream and ofstream.

2015-09-25 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38776/#review100707 --- Patch looks great! Reviews applied: [38775, 38776] All tests

Re: Review Request 38756: CMake: Add support for compiling the agent with CMake.

2015-09-25 Thread Joseph Wu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38756/#review100714 --- For me, `mkdir build && cd build && cmake .. && make`. Gave me an

Re: Review Request 38756: CMake: Add support for compiling the agent with CMake.

2015-09-25 Thread Alex Clemmer
> On Sept. 25, 2015, 11:16 p.m., Joseph Wu wrote: > > For me, `mkdir build && cd build && cmake .. && make`. > > > > Gave me an error (OSX): > > > > ``` > > Scanning dependencies of target mesos-0.25.0 > > /Applications/Xcode.app/Contents/Developer/usr/bin/make -f > >

Review Request 38775: Removed unnecessary close calls on ifstream and ofstream.

2015-09-25 Thread Ben Mahler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38775/ --- Review request for mesos and Jie Yu. Repository: mesos Description ---

  1   2   >