You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Alex Clemmer <cl...@gmail.com> on 2015/09/21 04:40:59 UTC
Review Request 38539: [VIA HAOSDENT] [1/2]Add CMake macro
VsBuildCommand in libprocess.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38539/
-----------------------------------------------------------
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 38539: [VIA HAOSDENT] [1/2]Add CMake macro
VsBuildCommand in libprocess.
Posted by Alex Clemmer <cl...@gmail.com>.
> On Sept. 25, 2015, 4:46 p.m., Artem Harutyunyan wrote:
> > 3rdparty/libprocess/3rdparty/CMakeLists.txt, line 242
> > <https://reviews.apache.org/r/38539/diff/2/?file=1078407#file1078407line242>
> >
> > This surpasses 80 character limit, is there a way to fix it here and elsewhere in this file?
I chose not to do this because the Google C++ style guide seems to say that raw string literals can go over 80 characters. Per our conversation, hopefully Joris can give guidance here?
- Alex
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38539/#review100596
-----------------------------------------------------------
On Sept. 21, 2015, 6:14 p.m., Alex Clemmer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38539/
> -----------------------------------------------------------
>
> (Updated Sept. 21, 2015, 6:14 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 38539: [VIA HAOSDENT] [1/2]Add CMake macro
VsBuildCommand in libprocess.
Posted by Alex Clemmer <cl...@gmail.com>.
> On Sept. 25, 2015, 4:46 p.m., Artem Harutyunyan wrote:
> > 3rdparty/libprocess/3rdparty/CMakeLists.txt, line 242
> > <https://reviews.apache.org/r/38539/diff/2/?file=1078407#file1078407line242>
> >
> > This surpasses 80 character limit, is there a way to fix it here and elsewhere in this file?
>
> Alex Clemmer wrote:
> I chose not to do this because the Google C++ style guide seems to say that raw string literals can go over 80 characters. Per our conversation, hopefully Joris can give guidance here?
Per conversation with Joris, this should be `nolint`'d.
> On Sept. 25, 2015, 4:46 p.m., Artem Harutyunyan wrote:
> > 3rdparty/libprocess/cmake/macros/VsBuildCommand.bat, line 34
> > <https://reviews.apache.org/r/38539/diff/2/?file=1078408#file1078408line34>
> >
> > Ditto.
The solution to this problem is to add a carot in a particular place on this line, so that we can split it up. Any other place seems to cause compilation to fail. The joy of batch scripting!
(Though, in batch's defense, bash is definitely not a better language.)
- Alex
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38539/#review100596
-----------------------------------------------------------
On Sept. 21, 2015, 6:14 p.m., Alex Clemmer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38539/
> -----------------------------------------------------------
>
> (Updated Sept. 21, 2015, 6:14 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 38539: [VIA HAOSDENT] [1/2]Add CMake macro
VsBuildCommand in libprocess.
Posted by Alex Clemmer <cl...@gmail.com>.
> On Sept. 25, 2015, 4:46 p.m., Artem Harutyunyan wrote:
> > 3rdparty/libprocess/3rdparty/CMakeLists.txt, line 242
> > <https://reviews.apache.org/r/38539/diff/2/?file=1078407#file1078407line242>
> >
> > This surpasses 80 character limit, is there a way to fix it here and elsewhere in this file?
>
> Alex Clemmer wrote:
> I chose not to do this because the Google C++ style guide seems to say that raw string literals can go over 80 characters. Per our conversation, hopefully Joris can give guidance here?
>
> Alex Clemmer wrote:
> Per conversation with Joris, this should be `nolint`'d.
This doesn't seem to be nolint'd because we don't lint CMakeLists.
- Alex
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38539/#review100596
-----------------------------------------------------------
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 38539: [VIA HAOSDENT] [1/2]Add CMake macro
VsBuildCommand in libprocess.
Posted by Artem Harutyunyan <ar...@mesosphere.io>.
> On Sept. 25, 2015, 9:46 a.m., Artem Harutyunyan wrote:
> >
>
> haosdent huang wrote:
> I don't think so. And Makefile.am also not follow the 80 characters limit.
I'd rather have the Makefile.am fixed too.
- Artem
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38539/#review100596
-----------------------------------------------------------
On Sept. 25, 2015, 3: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, 3: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 38539: [VIA HAOSDENT] [1/2]Add CMake macro
VsBuildCommand in libprocess.
Posted by haosdent huang <ha...@gmail.com>.
> 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:
https://reviews.apache.org/r/38539/#review100596
-----------------------------------------------------------
On Sept. 21, 2015, 6:14 p.m., Alex Clemmer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38539/
> -----------------------------------------------------------
>
> (Updated Sept. 21, 2015, 6:14 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 38539: [VIA HAOSDENT] [1/2]Add CMake macro
VsBuildCommand in libprocess.
Posted by Artem Harutyunyan <ar...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38539/#review100596
-----------------------------------------------------------
3rdparty/libprocess/3rdparty/CMakeLists.txt (line 207)
<https://reviews.apache.org/r/38539/#comment157821>
This surpasses 80 character limit, is there a way to fix it here and elsewhere in this file?
3rdparty/libprocess/cmake/macros/VsBuildCommand.bat (line 34)
<https://reviews.apache.org/r/38539/#comment157823>
Ditto.
3rdparty/libprocess/cmake/macros/VsBuildCommand.cmake (line 31)
<https://reviews.apache.org/r/38539/#comment157822>
Can we break this string so the line does not surpass 80 characaters?
- Artem Harutyunyan
On Sept. 21, 2015, 11:14 a.m., Alex Clemmer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38539/
> -----------------------------------------------------------
>
> (Updated Sept. 21, 2015, 11:14 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
> -------
>
> 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 38539: [VIA HAOSDENT] [1/2]Add CMake macro
VsBuildCommand in libprocess.
Posted by Artem Harutyunyan <ar...@mesosphere.io>.
-----------------------------------------------------------
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 Sept. 21, 2015, 11:14 a.m., Alex Clemmer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38539/
> -----------------------------------------------------------
>
> (Updated Sept. 21, 2015, 11:14 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
> -------
>
> 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 38539: [VIA HAOSDENT] [1/2]Add CMake macro
VsBuildCommand in libprocess.
Posted by haosdent huang <ha...@gmail.com>.
> On Sept. 26, 2015, 6:08 a.m., Joris Van Remoortere wrote:
> > hausdorff will follow up with a JIRA for preventing 32 bit builds: MESOS-3525
> > haosdent will follow up with better documentation for VsBuildCommand.bat
I create a issue here. https://issues.apache.org/jira/browse/MESOS-3528
- haosdent
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38539/#review100729
-----------------------------------------------------------
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 38539: [VIA HAOSDENT] [1/2]Add CMake macro
VsBuildCommand in libprocess.
Posted by Joris Van Remoortere <jo...@gmail.com>.
-----------------------------------------------------------
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 38539: [VIA HAOSDENT] [1/2]Add CMake macro
VsBuildCommand in libprocess.
Posted by Alex Clemmer <cl...@gmail.com>.
-----------------------------------------------------------
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 (updated)
-----
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 38539: [VIA HAOSDENT] [1/2]Add CMake macro
VsBuildCommand in libprocess.
Posted by Alex Clemmer <cl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38539/
-----------------------------------------------------------
(Updated Sept. 21, 2015, 6:14 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 (updated)
-----
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 38539: [VIA HAOSDENT] [1/2]Add CMake macro
VsBuildCommand in libprocess.
Posted by Alex Clemmer <cl...@gmail.com>.
> On Sept. 21, 2015, 6:08 p.m., Joseph Wu wrote:
> > 3rdparty/libprocess/cmake/macros/VsBuildCommand.bat, line 52
> > <https://reviews.apache.org/r/38539/diff/1/?file=1077772#file1077772line52>
> >
> > s/not/does not/
> > s/current/the current/
> >
> > And a period at the end.
I'd like to not change "current" to "the current" because that makes the comment go over 80 columns, and it doesn't add clarity.
The other changes have been applied though.
- Alex
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38539/#review99788
-----------------------------------------------------------
On Sept. 21, 2015, 2:40 a.m., Alex Clemmer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38539/
> -----------------------------------------------------------
>
> (Updated Sept. 21, 2015, 2:40 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
> -------
>
> 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 38539: [VIA HAOSDENT] [1/2]Add CMake macro
VsBuildCommand in libprocess.
Posted by Joseph Wu <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38539/#review99788
-----------------------------------------------------------
Ship it!
Definitely looks a lot cleaner with the NOOP macro :)
3rdparty/libprocess/cmake/macros/VsBuildCommand.bat (line 19)
<https://reviews.apache.org/r/38539/#comment156761>
Period at the end.
3rdparty/libprocess/cmake/macros/VsBuildCommand.bat (line 52)
<https://reviews.apache.org/r/38539/#comment156760>
s/not/does not/
s/current/the current/
And a period at the end.
- Joseph Wu
On Sept. 20, 2015, 7:40 p.m., Alex Clemmer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38539/
> -----------------------------------------------------------
>
> (Updated Sept. 20, 2015, 7:40 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
>
>