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

2015-09-26 Thread Joris Van Remoortere

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38551/#review100739
---

Ship it!


- Joris Van Remoortere


On Sept. 21, 2015, 6:30 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38551/
> ---
> 
> (Updated Sept. 21, 2015, 6:30 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Bugs: MESOS-3383
> https://issues.apache.org/jira/browse/MESOS-3383
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> [1/2]CMake: Add version info for APR we need to build Windows.
> 
> 
> Diffs
> -
> 
>   3rdparty/cmake/Versions.cmake PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38551/diff/
> 
> 
> Testing
> ---
> 
> Compiled and ran made sure libprocess and stout tests ran and passed on the 
> following platforms:
> 
> * OS X 10.10
> * Windows 10
> * Ubuntu 14.04.2
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>



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

2015-09-26 Thread Joris Van Remoortere

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38529/#review100722
---

Ship it!


- Joris Van Remoortere


On Sept. 20, 2015, 2:07 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38529/
> ---
> 
> (Updated Sept. 20, 2015, 2:07 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Bugs: MESOS-3345
> https://issues.apache.org/jira/browse/MESOS-3345
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> CMake: Only compile proc_tests.cpp for Linux platforms.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 
> baa648aacfd5204c33e102b58126862729fbb612 
> 
> Diff: https://reviews.apache.org/r/38529/diff/
> 
> 
> Testing
> ---
> 
> Compiled and ran made sure libprocess and stout tests ran and passed on the 
> following platforms:
> 
> * OS X 10.10
> * Windows 10
> * Ubuntu 14.04.2
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>



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

2015-09-26 Thread Alex Clemmer

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38753/
---

(Updated Sept. 26, 2015, 6 a.m.)


Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
Wu.


Repository: mesos


Description
---

CMake: Transition Stout tests to use new third-party build scripts.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/cmake/StoutTestsConfigure.cmake 
08325297ceb79b80c305ba4f2164ffd37591a0e8 

Diff: https://reviews.apache.org/r/38753/diff/


Testing
---

Compiled and ran made sure libprocess and stout tests ran and passed on the 
following platforms:

* OS X 10.10
* Ubuntu 14.04.2


Thanks,

Alex Clemmer



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

2015-09-26 Thread Joris Van Remoortere

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38540/#review100731
---

Ship it!


- Joris Van Remoortere


On Sept. 21, 2015, 2:41 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38540/
> ---
> 
> (Updated Sept. 21, 2015, 2:41 a.m.)
> 
> 
> Review request for mesos, haosdent huang, Artem Harutyunyan, Joris Van 
> Remoortere, and Joseph Wu.
> 
> 
> Bugs: MESOS-3434
> https://issues.apache.org/jira/browse/MESOS-3434
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Generate make batch file to build project in windows.
> 
> This commit is the product of two reviews; the first one[1], by
> haosdent, was lightly rebased by hausdorff to target a later HEAD, and
> then augmented to fix the broken Ubuntu 14.04 build that this commit
> caused. Other than those things, the review is functionally identical to
> the original, and all credit for the work should go to haosdent.
> 
> [1] Original review: https://reviews.apache.org/r/37275
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 3b6f4af337466d33cb915959a5995e4307b27be3 
>   cmake/MesosConfigure.cmake b530da4c1e6f202b682ad7d6892da95d2181f8c8 
> 
> Diff: https://reviews.apache.org/r/38540/diff/
> 
> 
> Testing
> ---
> 
> Compiled and ran made sure libprocess and stout tests ran and passed on the 
> following platforms:
> 
> * OS X 10.10
> * Windows 10
> * Ubuntu 14.04.2
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>



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

2015-09-26 Thread Joris Van Remoortere

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38539/#review100729
---

Ship it!


hausdorff will follow up with a JIRA for preventing 32 bit builds: MESOS-3525
haosdent will follow up with better documentation for VsBuildCommand.bat

- Joris Van Remoortere


On Sept. 25, 2015, 10:08 p.m., Alex Clemmer wrote:
> 
> ---
> 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, Artem Harutyunyan, Joris Van 
> Remoortere, and Joseph Wu.
> 
> 
> Bugs: MESOS-3434
> https://issues.apache.org/jira/browse/MESOS-3434
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add CMake macro VsBuildCommand in libprocess.
> 
> This commit is the product of two reviews; the first one[1], by
> haosdent, was lightly rebased by hausdorff to target a later HEAD, and
> then augmented to fix the broken Ubuntu 14.04 build that this commit
> caused. Other than those things, the review is functionally identical to
> the original, and all credit for the work should go to haosdent.
> 
> [1] Original review: https://reviews.apache.org/r/37273
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/CMakeLists.txt 
> b9c9fae7d448906e9c9f5ab0ee3fe138a0171a7d 
>   3rdparty/libprocess/cmake/macros/VsBuildCommand.bat PRE-CREATION 
>   3rdparty/libprocess/cmake/macros/VsBuildCommand.cmake PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38539/diff/
> 
> 
> Testing
> ---
> 
> Compiled and ran made sure libprocess and stout tests ran and passed on the 
> following platforms:
> 
> * OS X 10.10
> * Windows 10
> * Ubuntu 14.04.2
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>



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

2015-09-26 Thread Joris Van Remoortere

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38538/#review100726
---

Ship it!


- Joris Van Remoortere


On Sept. 21, 2015, 2:33 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38538/
> ---
> 
> (Updated Sept. 21, 2015, 2:33 a.m.)
> 
> 
> Review request for mesos, haosdent huang, Artem Harutyunyan, Joris Van 
> Remoortere, and Joseph Wu.
> 
> 
> Bugs: MESOS-3434
> https://issues.apache.org/jira/browse/MESOS-3434
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This was originally part of this review[1] by haosdent. It was split out
> and moved into the root folder by hausdorff, because we will later reuse
> it to build other third-party libraries on Windows that are not only in
> libprocess, and hence it belongs outside of the commits that touch
> libprocess.
> 
> But, the code is essentially haosdent's, so the credit should go to him.
> 
> [1] Original review: https://reviews.apache.org/r/37273
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 3b6f4af337466d33cb915959a5995e4307b27be3 
>   cmake/Common.cmake PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38538/diff/
> 
> 
> Testing
> ---
> 
> Compiled and ran made sure libprocess and stout tests ran and passed on the 
> following platforms:
> 
> * OS X 10.10
> * Windows 10
> * Ubuntu 14.04.2
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>



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

2015-09-26 Thread Joris Van Remoortere

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38751/#review100741
---

Ship it!



3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake (line 82)


Do we not need lflags for libevent?


- Joris Van Remoortere


On Sept. 25, 2015, 10:11 p.m., Alex Clemmer wrote:
> 
> ---
> 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 Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> CMake: Pull third-party configuration logic into its own .cmake file.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/CMakeLists.txt 
> b9c9fae7d448906e9c9f5ab0ee3fe138a0171a7d 
>   3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake PRE-CREATION 
>   3rdparty/libprocess/cmake/ProcessConfigure.cmake 
> a5f8d399e151acad87bb72ecb1f7372b2c467423 
>   3rdparty/libprocess/cmake/ProcessTestsConfigure.cmake 
> 9e4dcb83a8cc4e95a2a38573944f6b38e2eac76e 
> 
> Diff: https://reviews.apache.org/r/38751/diff/
> 
> 
> Testing
> ---
> 
> Compiled and ran made sure libprocess and stout tests ran and passed on the 
> following platforms:
> 
> * OS X 10.10
> * Ubuntu 14.04.2
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>



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

2015-09-26 Thread Joris Van Remoortere

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38549/#review100735
---

Ship it!


This is a big decision because on linux right now the default is libev. The 
code paths are significantly different between libev and libevent.
hausdorff is going to follow up with a JIRA for this, and force users to 
compile with the --enable-libevent flag. It will error otherwise. This is a 
nice compromise to ensure that users understand the decision point.


3rdparty/cmake/Versions.cmake (line 7)


todo for compiling non-beta version of libevent



CMakeLists.txt (line 37)


comment about difference of default from linux


- Joris Van Remoortere


On Sept. 21, 2015, 6:31 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38549/
> ---
> 
> (Updated Sept. 21, 2015, 6:31 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Bugs: MESOS-3380
> https://issues.apache.org/jira/browse/MESOS-3380
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Mesos will need to use libevent to build the process library on Windows.
> 
> This commit will add a default version of libevent, which we will
> eventually retrieve from the 3rdparty GitHub repository, which for now
> is our "official" distribution channel for out-of-band third-party
> dependencies (especially on Windows, which has no package manager).
> 
> 
> Diffs
> -
> 
>   3rdparty/cmake/Versions.cmake PRE-CREATION 
>   CMakeLists.txt 3b6f4af337466d33cb915959a5995e4307b27be3 
> 
> Diff: https://reviews.apache.org/r/38549/diff/
> 
> 
> Testing
> ---
> 
> Compiled and ran made sure libprocess and stout tests ran and passed on the 
> following platforms:
> 
> * OS X 10.10
> * Windows 10
> * Ubuntu 14.04.2
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>



Re: Review Request 38457: CMake: Fix MESOS-3250, a dynamic load error in Stout tests on OS X.

2015-09-26 Thread Joris Van Remoortere

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38457/#review100721
---

Ship it!


- Joris Van Remoortere


On Sept. 21, 2015, 2:43 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38457/
> ---
> 
> (Updated Sept. 21, 2015, 2:43 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Bugs: MESOS-3250
> https://issues.apache.org/jira/browse/MESOS-3250
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> A few third-party libraries (libev, gmock) do not have `make install`
> commands available, so below we have to add our own "install" commands.
> 
> The reason is: if we do not, we get runtime library load problems on OS
> X. In particular, `dydl` will look for these libraries at the prefix we
> passed to `configure` (or in `/usr/local` if we did not pass a prefix
> in), but since they don't have a `make install` step, they never get
> placed in the prefix folder.
> 
> Our solution is to:
>   (1) make a lib directory inside the Mesos folder for each of the
>   libraries that has no install step, and
>   (2) copy all such libraries into their respective directories.
> 
> (Note that step (1) is not only convenient, but important: make will add
> a `lib` to the end of your prefix path when linking, and since the built
> libraries end up in a `.libs` folder, it's not enough to simply pass the
> build directory into `configure` as a prefix; so if we're going to move
> the libraries, we might as well move them to a library folder.)
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/CMakeLists.txt 
> b9c9fae7d448906e9c9f5ab0ee3fe138a0171a7d 
> 
> Diff: https://reviews.apache.org/r/38457/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>



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

2015-09-26 Thread Joris Van Remoortere

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38542/#review100733
---

Ship it!


This is great Alex!

- Joris Van Remoortere


On Sept. 21, 2015, 3:23 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38542/
> ---
> 
> (Updated Sept. 21, 2015, 3:23 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently we configure the version information of third-party
> dependencies in the CMake build system from magic strings.
> 
> This commit will transition away from the magic string solution and
> towards the variables we define in `Versions.cmake`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/cmake/ProcessConfigure.cmake 
> a5f8d399e151acad87bb72ecb1f7372b2c467423 
>   3rdparty/libprocess/cmake/ProcessTestsConfigure.cmake 
> 9e4dcb83a8cc4e95a2a38573944f6b38e2eac76e 
> 
> Diff: https://reviews.apache.org/r/38542/diff/
> 
> 
> Testing
> ---
> 
> Compiled and ran made sure libprocess and stout tests ran and passed on the 
> following platforms:
> 
> * OS X 10.10
> * Windows 10
> * Ubuntu 14.04.2
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>



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

2015-09-26 Thread Joris Van Remoortere

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38552/#review100740
---

Ship it!


- Joris Van Remoortere


On Sept. 21, 2015, 11:21 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38552/
> ---
> 
> (Updated Sept. 21, 2015, 11:21 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Bugs: MESOS-3383
> https://issues.apache.org/jira/browse/MESOS-3383
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> APR is required for Mesos to run. Normally we expect that a user has
> obtained APR with their favorite package manager, but on Windows, we
> cannot really expect that this is the case. So, we have to rope in the
> dependency manually in the CMake build system.
> 
> This commit will introduce the necessary build targets to obtain,
> configure, build, and install APR for Windows builds. It will ignore APR
> and assume it's there for Linux builds.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/CMakeLists.txt 
> b9c9fae7d448906e9c9f5ab0ee3fe138a0171a7d 
>   3rdparty/libprocess/cmake/ProcessConfigure.cmake 
> a5f8d399e151acad87bb72ecb1f7372b2c467423 
> 
> Diff: https://reviews.apache.org/r/38552/diff/
> 
> 
> Testing
> ---
> 
> Compiled and ran made sure libprocess and stout tests ran and passed on the 
> following platforms:
> 
> * OS X 10.10
> * Windows 10
> * Ubuntu 14.04.2
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>



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

2015-09-26 Thread Joris Van Remoortere

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38550/#review100738
---

Ship it!


- Joris Van Remoortere


On Sept. 21, 2015, 10:43 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38550/
> ---
> 
> (Updated Sept. 21, 2015, 10:43 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Bugs: MESOS-3380
> https://issues.apache.org/jira/browse/MESOS-3380
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Since there is no robust standalone libev support on Windows, Mesos will
> need to use libevent to support Windows, at least in the short term.
> 
> This commit will add logic to configure, build, and install libevent for
> all Windows builds.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/CMakeLists.txt 
> b9c9fae7d448906e9c9f5ab0ee3fe138a0171a7d 
>   3rdparty/libprocess/cmake/ProcessConfigure.cmake 
> a5f8d399e151acad87bb72ecb1f7372b2c467423 
> 
> Diff: https://reviews.apache.org/r/38550/diff/
> 
> 
> Testing
> ---
> 
> Compiled and ran made sure libprocess and stout tests ran and passed on the 
> following platforms:
> 
> * OS X 10.10
> * Windows 10
> * Ubuntu 14.04.2
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>



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

2015-09-26 Thread Joris Van Remoortere

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38541/#review100732
---

Ship it!


- Joris Van Remoortere


On Sept. 25, 2015, 10:09 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38541/
> ---
> 
> (Updated Sept. 25, 2015, 10:09 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> As we prepare to add more third-party dependencies, it will be helpful
> to have all the version information of our third-party dependencies in
> one place, rather than scattered everywhere in the project.
> 
> This commit will introduce `Versions.cmake`, which will hold all this
> information.
> 
> 
> Diffs
> -
> 
>   3rdparty/cmake/Versions.cmake PRE-CREATION 
>   CMakeLists.txt 3b6f4af337466d33cb915959a5995e4307b27be3 
> 
> Diff: https://reviews.apache.org/r/38541/diff/
> 
> 
> Testing
> ---
> 
> Compiled and ran made sure libprocess and stout tests ran and passed on the 
> following platforms:
> 
> * OS X 10.10
> * Windows 10
> * Ubuntu 14.04.2
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>



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

2015-09-26 Thread Joris Van Remoortere

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38531/#review100725
---

Ship it!


- Joris Van Remoortere


On Sept. 20, 2015, 2:07 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38531/
> ---
> 
> (Updated Sept. 20, 2015, 2:07 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Bugs: MESOS-3345
> https://issues.apache.org/jira/browse/MESOS-3345
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> CMake: Update CMake config to build Mesos against picojson v1.3.0.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/cmake/ProcessConfigure.cmake 
> a5f8d399e151acad87bb72ecb1f7372b2c467423 
> 
> Diff: https://reviews.apache.org/r/38531/diff/
> 
> 
> Testing
> ---
> 
> Compiled and ran made sure libprocess and stout tests ran and passed on the 
> following platforms:
> 
> * OS X 10.10
> * Windows 10
> * Ubuntu 14.04.2
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>



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

2015-09-26 Thread Alex Clemmer

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38756/
---

(Updated Sept. 26, 2015, 10:05 p.m.)


Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
Wu.


Bugs: MESOS-3385
https://issues.apache.org/jira/browse/MESOS-3385


Repository: mesos


Description
---

CMake: Add support for compiling the agent with CMake.


Diffs (updated)
-

  CMakeLists.txt 3b6f4af337466d33cb915959a5995e4307b27be3 
  cmake/MesosConfigure.cmake b530da4c1e6f202b682ad7d6892da95d2181f8c8 
  src/CMakeLists.txt PRE-CREATION 
  src/cmake/MesosProtobuf.cmake PRE-CREATION 
  src/slave/cmake/SlaveConfigure.cmake PRE-CREATION 

Diff: https://reviews.apache.org/r/38756/diff/


Testing
---

Compiled and ran made sure libprocess and stout tests ran and passed on the 
following platforms:

* OS X 10.10
* Ubuntu 14.04.2


Thanks,

Alex Clemmer



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

2015-09-26 Thread Alex Clemmer

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38751/
---

(Updated Sept. 26, 2015, 10:05 p.m.)


Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
Wu.


Repository: mesos


Description
---

CMake: Pull third-party configuration logic into its own .cmake file.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/CMakeLists.txt 
b9c9fae7d448906e9c9f5ab0ee3fe138a0171a7d 
  3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake PRE-CREATION 
  3rdparty/libprocess/cmake/ProcessConfigure.cmake 
a5f8d399e151acad87bb72ecb1f7372b2c467423 
  3rdparty/libprocess/cmake/ProcessTestsConfigure.cmake 
9e4dcb83a8cc4e95a2a38573944f6b38e2eac76e 

Diff: https://reviews.apache.org/r/38751/diff/


Testing
---

Compiled and ran made sure libprocess and stout tests ran and passed on the 
following platforms:

* OS X 10.10
* Ubuntu 14.04.2


Thanks,

Alex Clemmer



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

2015-09-26 Thread Alex Clemmer

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38551/
---

(Updated Sept. 26, 2015, 10:05 p.m.)


Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
Wu.


Bugs: MESOS-3383
https://issues.apache.org/jira/browse/MESOS-3383


Repository: mesos


Description
---

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


Diffs (updated)
-

  3rdparty/cmake/Versions.cmake PRE-CREATION 

Diff: https://reviews.apache.org/r/38551/diff/


Testing
---

Compiled and ran made sure libprocess and stout tests ran and passed on the 
following platforms:

* OS X 10.10
* Windows 10
* Ubuntu 14.04.2


Thanks,

Alex Clemmer



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

2015-09-26 Thread Alex Clemmer

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38552/
---

(Updated Sept. 26, 2015, 10:05 p.m.)


Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
Wu.


Bugs: MESOS-3383
https://issues.apache.org/jira/browse/MESOS-3383


Repository: mesos


Description
---

APR is required for Mesos to run. Normally we expect that a user has
obtained APR with their favorite package manager, but on Windows, we
cannot really expect that this is the case. So, we have to rope in the
dependency manually in the CMake build system.

This commit will introduce the necessary build targets to obtain,
configure, build, and install APR for Windows builds. It will ignore APR
and assume it's there for Linux builds.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/CMakeLists.txt 
b9c9fae7d448906e9c9f5ab0ee3fe138a0171a7d 
  3rdparty/libprocess/cmake/ProcessConfigure.cmake 
a5f8d399e151acad87bb72ecb1f7372b2c467423 

Diff: https://reviews.apache.org/r/38552/diff/


Testing
---

Compiled and ran made sure libprocess and stout tests ran and passed on the 
following platforms:

* OS X 10.10
* Windows 10
* Ubuntu 14.04.2


Thanks,

Alex Clemmer



Re: Review Request 38752: CMake: Update MesosConfigure to use new process configure scripts.

2015-09-26 Thread Alex Clemmer

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38752/
---

(Updated Sept. 26, 2015, 10:05 p.m.)


Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
Wu.


Repository: mesos


Description
---

CMake: Transition Mesos to use new third-party build scripts.


Diffs (updated)
-

  cmake/MesosConfigure.cmake b530da4c1e6f202b682ad7d6892da95d2181f8c8 

Diff: https://reviews.apache.org/r/38752/diff/


Testing
---

Compiled and ran made sure libprocess and stout tests ran and passed on the 
following platforms:

* OS X 10.10
* Ubuntu 14.04.2


Thanks,

Alex Clemmer



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

2015-09-26 Thread Alex Clemmer

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38754/
---

(Updated Sept. 26, 2015, 10:05 p.m.)


Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
Wu.


Bugs: MESOS-3381
https://issues.apache.org/jira/browse/MESOS-3381


Repository: mesos


Description
---

CMake: Add build/configure/install logic for Zookeeper.


Diffs (updated)
-

  3rdparty/CMakeLists.txt PRE-CREATION 
  3rdparty/cmake/Mesos3rdpartyConfigure.cmake PRE-CREATION 
  3rdparty/cmake/Versions.cmake PRE-CREATION 
  CMakeLists.txt 3b6f4af337466d33cb915959a5995e4307b27be3 
  cmake/MesosConfigure.cmake b530da4c1e6f202b682ad7d6892da95d2181f8c8 

Diff: https://reviews.apache.org/r/38754/diff/


Testing
---

Compiled and ran made sure libprocess and stout tests ran and passed on the 
following platforms:

* OS X 10.10
* Ubuntu 14.04.2


Thanks,

Alex Clemmer



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

2015-09-26 Thread Alex Clemmer

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38755/
---

(Updated Sept. 26, 2015, 10:05 p.m.)


Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
Wu.


Repository: mesos


Description
---

CMake: Move compiler configuration logic to its own file.


Diffs (updated)
-

  cmake/CompilationConfigure.cmake PRE-CREATION 
  cmake/MesosConfigure.cmake b530da4c1e6f202b682ad7d6892da95d2181f8c8 

Diff: https://reviews.apache.org/r/38755/diff/


Testing
---

Compiled and ran made sure libprocess and stout tests ran and passed on the 
following platforms:

* OS X 10.10
* Ubuntu 14.04.2


Thanks,

Alex Clemmer



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

2015-09-26 Thread Alex Clemmer

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38753/
---

(Updated Sept. 26, 2015, 10:05 p.m.)


Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
Wu.


Repository: mesos


Description
---

CMake: Transition Stout tests to use new third-party build scripts.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/cmake/StoutTestsConfigure.cmake 
08325297ceb79b80c305ba4f2164ffd37591a0e8 

Diff: https://reviews.apache.org/r/38753/diff/


Testing
---

Compiled and ran made sure libprocess and stout tests ran and passed on the 
following platforms:

* OS X 10.10
* Ubuntu 14.04.2


Thanks,

Alex Clemmer



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

2015-09-26 Thread Neil Conway

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38774/#review100765
---


Is there already an RAII wrapper for os::close() we can use? If not, might be 
worth writing one.

- Neil Conway


On Sept. 25, 2015, 9 p.m., Chi Zhang wrote:
> 
> ---
> 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
> https://issues.apache.org/jira/browse/mesos-3519
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> state: fix file descriptor leak
> 
> 
> Diffs
> -
> 
>   src/slave/state.cpp 47c66dc80d57db86981769d404c2c8c7c972fec0 
> 
> Diff: https://reviews.apache.org/r/38774/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Chi Zhang
> 
>



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

2015-09-26 Thread Alex Clemmer

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38756/
---

(Updated Sept. 26, 2015, 10:57 p.m.)


Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
Wu.


Bugs: MESOS-3385
https://issues.apache.org/jira/browse/MESOS-3385


Repository: mesos


Description
---

CMake: Add support for compiling the agent with CMake.


Diffs (updated)
-

  CMakeLists.txt 3b6f4af337466d33cb915959a5995e4307b27be3 
  cmake/MesosConfigure.cmake b530da4c1e6f202b682ad7d6892da95d2181f8c8 
  src/CMakeLists.txt PRE-CREATION 
  src/cmake/MesosProtobuf.cmake PRE-CREATION 
  src/slave/cmake/SlaveConfigure.cmake PRE-CREATION 

Diff: https://reviews.apache.org/r/38756/diff/


Testing
---

Compiled and ran made sure libprocess and stout tests ran and passed on the 
following platforms:

* OS X 10.10
* Ubuntu 14.04.2


Thanks,

Alex Clemmer



Review Request 38793: CMake: Transition to CMake-based build system for GMock/GTest.

2015-09-26 Thread Alex Clemmer

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38793/
---

Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
Wu.


Repository: mesos


Description
---

CMake: Transition to CMake-based build system for GMock/GTest.


Diffs
-

  3rdparty/libprocess/3rdparty/CMakeLists.txt 
b9c9fae7d448906e9c9f5ab0ee3fe138a0171a7d 
  3rdparty/libprocess/cmake/ProcessTestsConfigure.cmake 
9e4dcb83a8cc4e95a2a38573944f6b38e2eac76e 

Diff: https://reviews.apache.org/r/38793/diff/


Testing
---

Compiled and ran made sure libprocess and stout tests ran and passed on the 
following platforms:

* OS X 10.10
* Ubuntu 14.04.2


Thanks,

Alex Clemmer



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

2015-09-26 Thread Alex Clemmer


> On Sept. 26, 2015, 6:09 a.m., Joris Van Remoortere wrote:
> > This is a big decision because on linux right now the default is libev. The 
> > code paths are significantly different between libev and libevent.
> > hausdorff is going to follow up with a JIRA for this, and force users to 
> > compile with the --enable-libevent flag. It will error otherwise. This is a 
> > nice compromise to ensure that users understand the decision point.

A small note about what's going on here. This last revision will allow the user 
to pass in a `-DENABLE_LIBEVENT=1` flag and compile with libevent. The catch 
is, I forgot to make a few things condition on `ENABLE_LIBEVENT` (such as the 
`-lev` flag. So, rather than making those changes and threading it through like 
9 commits as this code moves around and gets altered, I've added a CMake error 
that will throw an error if you try to use libevent on Unix, and I'll push a 
patch up today that actually handles this correctly, which will apply after all 
these patches go in.

Does that make sense?


- Alex


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38549/#review100735
---


On Sept. 26, 2015, 9:56 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38549/
> ---
> 
> (Updated Sept. 26, 2015, 9:56 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Bugs: MESOS-3380
> https://issues.apache.org/jira/browse/MESOS-3380
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Mesos will need to use libevent to build the process library on Windows.
> 
> This commit will add a default version of libevent, which we will
> eventually retrieve from the 3rdparty GitHub repository, which for now
> is our "official" distribution channel for out-of-band third-party
> dependencies (especially on Windows, which has no package manager).
> 
> 
> Diffs
> -
> 
>   3rdparty/cmake/Versions.cmake PRE-CREATION 
>   CMakeLists.txt 3b6f4af337466d33cb915959a5995e4307b27be3 
> 
> Diff: https://reviews.apache.org/r/38549/diff/
> 
> 
> Testing
> ---
> 
> Compiled and ran made sure libprocess and stout tests ran and passed on the 
> following platforms:
> 
> * OS X 10.10
> * Windows 10
> * Ubuntu 14.04.2
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>



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

2015-09-26 Thread Alex Clemmer

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38550/
---

(Updated Sept. 26, 2015, 10:03 p.m.)


Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
Wu.


Bugs: MESOS-3380
https://issues.apache.org/jira/browse/MESOS-3380


Repository: mesos


Description
---

Since there is no robust standalone libev support on Windows, Mesos will
need to use libevent to support Windows, at least in the short term.

This commit will add logic to configure, build, and install libevent for
all Windows builds.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/CMakeLists.txt 
b9c9fae7d448906e9c9f5ab0ee3fe138a0171a7d 
  3rdparty/libprocess/cmake/ProcessConfigure.cmake 
a5f8d399e151acad87bb72ecb1f7372b2c467423 

Diff: https://reviews.apache.org/r/38550/diff/


Testing
---

Compiled and ran made sure libprocess and stout tests ran and passed on the 
following platforms:

* OS X 10.10
* Windows 10
* Ubuntu 14.04.2


Thanks,

Alex Clemmer



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

2015-09-26 Thread Alex Clemmer


> On Sept. 25, 2015, 7:07 a.m., Alex Clemmer wrote:
> > cmake/CompilationConfigure.cmake, line 97
> > 
> >
> > Oh yeah. I forgot to remove these. I don't actually know what 
> > `pkglibexecdir` does in autoconf; I'll look it up tomorrow if no one 
> > mentions it before I get back to these reviews. :)

Please pay close attention to this. I only _think_ I got it right.


- Alex


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38755/#review100554
---


On Sept. 26, 2015, 10:05 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38755/
> ---
> 
> (Updated Sept. 26, 2015, 10:05 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> CMake: Move compiler configuration logic to its own file.
> 
> 
> Diffs
> -
> 
>   cmake/CompilationConfigure.cmake PRE-CREATION 
>   cmake/MesosConfigure.cmake b530da4c1e6f202b682ad7d6892da95d2181f8c8 
> 
> Diff: https://reviews.apache.org/r/38755/diff/
> 
> 
> Testing
> ---
> 
> Compiled and ran made sure libprocess and stout tests ran and passed on the 
> following platforms:
> 
> * OS X 10.10
> * Ubuntu 14.04.2
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>



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

2015-09-26 Thread Alex Clemmer


> On Sept. 26, 2015, 6:10 a.m., Joris Van Remoortere wrote:
> > 3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake, line 82
> > 
> >
> > Do we not need lflags for libevent?
> 
> Alex Clemmer wrote:
> The short answer is that we didn't need this because we had only intended 
> this to work on Windows. This will change with the next set of revisions, 
> because we have decided we want to make the user opt into libevent when we 
> build on Windows, which requires (obviously) wiring up the flag to be 
> user-facing. Until now, the flag wasn't user-facing, so we never actually hit 
> this code path. Now that it is, we will correctly handle this too. :)

So I'll address this in the next patch. This would be too annoying to thread 
through and rebase all the commits where this code is moving around or getting 
altered. It's a trivial patch to do.


- Alex


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38751/#review100741
---


On Sept. 26, 2015, 10:05 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38751/
> ---
> 
> (Updated Sept. 26, 2015, 10:05 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> CMake: Pull third-party configuration logic into its own .cmake file.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/CMakeLists.txt 
> b9c9fae7d448906e9c9f5ab0ee3fe138a0171a7d 
>   3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake PRE-CREATION 
>   3rdparty/libprocess/cmake/ProcessConfigure.cmake 
> a5f8d399e151acad87bb72ecb1f7372b2c467423 
>   3rdparty/libprocess/cmake/ProcessTestsConfigure.cmake 
> 9e4dcb83a8cc4e95a2a38573944f6b38e2eac76e 
> 
> Diff: https://reviews.apache.org/r/38751/diff/
> 
> 
> Testing
> ---
> 
> Compiled and ran made sure libprocess and stout tests ran and passed on the 
> following platforms:
> 
> * OS X 10.10
> * Ubuntu 14.04.2
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>



Re: Review Request 38793: CMake: Transition to CMake-based build system for GMock/GTest.

2015-09-26 Thread Mesos ReviewBot

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38793/#review100767
---


Patch looks great!

Reviews applied: [38456, 38457, 38529, 38530, 38531, 38538, 38539, 38540, 
38541, 38542, 38549, 38550, 38551, 38552, 38751, 38752, 38753, 38754, 38755, 
38756, 38793]

All tests passed.

- Mesos ReviewBot


On Sept. 26, 2015, 10:58 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38793/
> ---
> 
> (Updated Sept. 26, 2015, 10:58 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> CMake: Transition to CMake-based build system for GMock/GTest.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/CMakeLists.txt 
> b9c9fae7d448906e9c9f5ab0ee3fe138a0171a7d 
>   3rdparty/libprocess/cmake/ProcessTestsConfigure.cmake 
> 9e4dcb83a8cc4e95a2a38573944f6b38e2eac76e 
> 
> Diff: https://reviews.apache.org/r/38793/diff/
> 
> 
> Testing
> ---
> 
> Compiled and ran made sure libprocess and stout tests ran and passed on the 
> following platforms:
> 
> * OS X 10.10
> * Ubuntu 14.04.2
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>



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

2015-09-26 Thread Mesos ReviewBot

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38705/#review100770
---


Patch looks great!

Reviews applied: [38705]

All tests passed.

- Mesos ReviewBot


On Sept. 27, 2015, 2:02 a.m., Artem Harutyunyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38705/
> ---
> 
> (Updated Sept. 27, 2015, 2:02 a.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere, Joseph Wu, and Vinod Kone.
> 
> 
> Bugs: MESOS-3468
> https://issues.apache.org/jira/browse/MESOS-3468
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   support/apply-reviews.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38705/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Artem Harutyunyan
> 
>



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

2015-09-26 Thread Klaus Ma

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38342/
---

(Updated Sept. 27, 2015, 1:34 a.m.)


Review request for mesos, Alexander Rukletsov, Michael Park, and Jan Schlicht.


Changes
---

Address comments:
1. Add inline to `Object protobuf(const google::protobuf::Message& message)`
2. keep `static_assert` to check error at compiling time
3. move the commnets outside the function


Bugs: MESOS-3405
https://issues.apache.org/jira/browse/MESOS-3405


Repository: mesos


Description
---

Currently, `stout/protobuf.hpp` provides a `JSON::Protobuf` utility which 
converts a `google::protobuf::Message` into a `JSON::Object`.
We should add the support for `google::protobuf::RepeatedPtrField` by 
introducing overloaded functions.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp 2285ce9 
  3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp 68328a2 
  3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.h 8ebb798 
  3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.cc 34eb6d0 
  3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.proto 920f5c9 

Diff: https://reviews.apache.org/r/38342/diff/


Testing
---

cd 3rdparty/libprocess/3rdparty/stout
./boostrap
./configure
make


Thanks,

Klaus Ma



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

2015-09-26 Thread Artem Harutyunyan


> On Sept. 24, 2015, 5:38 p.m., Joseph Wu wrote:
> > support/apply-reviews.py, line 13
> > 
> >
> > Might be cleaner/safer to use urlparse.urljoin for this:
> > https://docs.python.org/2/library/urlparse.html#urlparse.urljoin

`urlparse.urljoin` does not seem to do exactly what is needed here.


> On Sept. 24, 2015, 5:38 p.m., Joseph Wu wrote:
> > support/apply-reviews.py, line 68
> > 
> >
> > Seems like you don't need a `-r` in this case, because we'd only use 
> > this script for reviewboard.
> > 
> > i.e. s/-r/review/

I have it for compatibility with `apply-review.sh` and also in case we decide 
to add support for github later.


- Artem


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38705/#review100505
---


On Sept. 23, 2015, 9:26 p.m., Artem Harutyunyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38705/
> ---
> 
> (Updated Sept. 23, 2015, 9:26 p.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere, Joseph Wu, and Vinod Kone.
> 
> 
> Bugs: MESOS-3468
> https://issues.apache.org/jira/browse/MESOS-3468
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   support/apply-reviews.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38705/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Artem Harutyunyan
> 
>



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

2015-09-26 Thread Artem Harutyunyan

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38705/
---

(Updated Sept. 26, 2015, 7:02 p.m.)


Review request for mesos, Joris Van Remoortere, Joseph Wu, and Vinod Kone.


Changes
---

Addressed comments; changed tab size to 2 spaces.


Bugs: MESOS-3468
https://issues.apache.org/jira/browse/MESOS-3468


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  support/apply-reviews.py PRE-CREATION 

Diff: https://reviews.apache.org/r/38705/diff/


Testing
---


Thanks,

Artem Harutyunyan