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/10/23 10:57:45 UTC
Review Request 39583: Windows: Added `WindowsError` to parallel
`ErrnoError`.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39583/
-----------------------------------------------------------
Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
Repository: mesos
Description
-------
Windows: Added `WindowsError` to parallel `ErrnoError`.
Diffs
-----
3rdparty/libprocess/3rdparty/stout/include/Makefile.am ba2836a72ceee948cb43364e80ada9f132f33d04
3rdparty/libprocess/3rdparty/stout/include/stout/try.hpp fdd33512c8d8752093f72f597a7d647eb5e3c285
3rdparty/libprocess/3rdparty/stout/include/stout/windows/error.hpp PRE-CREATION
Diff: https://reviews.apache.org/r/39583/diff/
Testing
-------
`make check` from autotools on Ubuntu 15.
`make check` from CMake on OS X 10.10.
Ran `check` project in VS on Windows 10.
Thanks,
Alex Clemmer
Re: Review Request 39583: Windows: Added `WindowsError` to parallel
`ErrnoError`.
Posted by Alex Clemmer <cl...@gmail.com>.
> On Oct. 23, 2015, 9:15 a.m., Benjamin Bannier wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/windows/error.hpp, line 34
> > <https://reviews.apache.org/r/39583/diff/1/?file=1103900#file1103900line34>
> >
> > This doesn't use any instance data so could probably be either `const`, `static`, or a free function.
Ah, thanks, Benjamin! I'm a complete C++ noob, unforunately. :)
- Alex
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39583/#review103746
-----------------------------------------------------------
On Oct. 23, 2015, 8:57 a.m., Alex Clemmer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39583/
> -----------------------------------------------------------
>
> (Updated Oct. 23, 2015, 8:57 a.m.)
>
>
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Windows: Added `WindowsError` to parallel `ErrnoError`.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/include/Makefile.am ba2836a72ceee948cb43364e80ada9f132f33d04
> 3rdparty/libprocess/3rdparty/stout/include/stout/try.hpp fdd33512c8d8752093f72f597a7d647eb5e3c285
> 3rdparty/libprocess/3rdparty/stout/include/stout/windows/error.hpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/39583/diff/
>
>
> Testing
> -------
>
> `make check` from autotools on Ubuntu 15.
> `make check` from CMake on OS X 10.10.
> Ran `check` project in VS on Windows 10.
>
>
> Thanks,
>
> Alex Clemmer
>
>
Re: Review Request 39583: Windows: Added `WindowsError` to parallel
`ErrnoError`.
Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39583/#review103746
-----------------------------------------------------------
3rdparty/libprocess/3rdparty/stout/include/stout/windows/error.hpp (line 34)
<https://reviews.apache.org/r/39583/#comment161823>
This doesn't use any instance data so could probably be either `const`, `static`, or a free function.
- Benjamin Bannier
On Oct. 23, 2015, 8:57 a.m., Alex Clemmer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39583/
> -----------------------------------------------------------
>
> (Updated Oct. 23, 2015, 8:57 a.m.)
>
>
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Windows: Added `WindowsError` to parallel `ErrnoError`.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/include/Makefile.am ba2836a72ceee948cb43364e80ada9f132f33d04
> 3rdparty/libprocess/3rdparty/stout/include/stout/try.hpp fdd33512c8d8752093f72f597a7d647eb5e3c285
> 3rdparty/libprocess/3rdparty/stout/include/stout/windows/error.hpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/39583/diff/
>
>
> Testing
> -------
>
> `make check` from autotools on Ubuntu 15.
> `make check` from CMake on OS X 10.10.
> Ran `check` project in VS on Windows 10.
>
>
> Thanks,
>
> Alex Clemmer
>
>
Re: Review Request 39583: Windows: Added `WindowsError` to parallel
`ErrnoError`.
Posted by Alex Clemmer <cl...@gmail.com>.
> On Dec. 10, 2015, 2:48 a.m., Joris Van Remoortere wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/windows/error.hpp, line 21
> > <https://reviews.apache.org/r/39583/diff/4/?file=1125978#file1125978line21>
> >
> > Maybe we can drop the `Try` specific part of this comment.
This comment is copied from the Unix version, and it does appear to be true. So I recommend we keep it. But, if we want to change it, we should actually change it in both places.
- Alex
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39583/#review109665
-----------------------------------------------------------
On Dec. 10, 2015, 2:14 a.m., Alex Clemmer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39583/
> -----------------------------------------------------------
>
> (Updated Dec. 10, 2015, 2:14 a.m.)
>
>
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
>
>
> Bugs: MESOS-4110
> https://issues.apache.org/jira/browse/MESOS-4110
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Windows: Added `WindowsError` to parallel `ErrnoError`.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/include/Makefile.am a8c35c086ecae21701f6a720f25231c1b0d4e329
> 3rdparty/libprocess/3rdparty/stout/include/stout/try.hpp fdd33512c8d8752093f72f597a7d647eb5e3c285
> 3rdparty/libprocess/3rdparty/stout/include/stout/windows/error.hpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/39583/diff/
>
>
> Testing
> -------
>
> `make check` from autotools on Ubuntu 15.
> `make check` from CMake on OS X 10.10.
> Ran `check` project in VS on Windows 10.
>
>
> Thanks,
>
> Alex Clemmer
>
>
Re: Review Request 39583: Windows: Added `WindowsError` to parallel
`ErrnoError`.
Posted by Alex Clemmer <cl...@gmail.com>.
> On Dec. 10, 2015, 2:48 a.m., Joris Van Remoortere wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/try.hpp, lines 27-29
> > <https://reviews.apache.org/r/39583/diff/4/?file=1125977#file1125977line27>
> >
> > Let's move this into `<stout/error.hpp>`
Per discussion we actually move the error type to errorbase.hpp, and then have error.hpp #include both `errorbase.hpp` and `windows/error.hpp`.
- Alex
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39583/#review109665
-----------------------------------------------------------
On Dec. 12, 2015, 11:08 p.m., Alex Clemmer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39583/
> -----------------------------------------------------------
>
> (Updated Dec. 12, 2015, 11:08 p.m.)
>
>
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
>
>
> Bugs: MESOS-4110
> https://issues.apache.org/jira/browse/MESOS-4110
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Windows: Added `WindowsError` to parallel `ErrnoError`.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/include/Makefile.am a25e2c1e5584e744c666bbc654eafbfc5f7b10e6
> 3rdparty/libprocess/3rdparty/stout/include/stout/error.hpp 605c23bb9273f12dabbd2d6fa87ca0b06d9958ca
> 3rdparty/libprocess/3rdparty/stout/include/stout/errorbase.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/try.hpp 9e3b007d9f8db1e16c1f6928d85c230612741cc0
> 3rdparty/libprocess/3rdparty/stout/include/stout/windows/error.hpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/39583/diff/
>
>
> Testing
> -------
>
> `make check` from autotools on Ubuntu 15.
> `make check` from CMake on OS X 10.10.
> Ran `check` project in VS on Windows 10.
>
>
> Thanks,
>
> Alex Clemmer
>
>
Re: Review Request 39583: Windows: Added `WindowsError` to parallel
`ErrnoError`.
Posted by Joris Van Remoortere <jo...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39583/#review109665
-----------------------------------------------------------
Ship it!
3rdparty/libprocess/3rdparty/stout/include/stout/try.hpp (lines 27 - 29)
<https://reviews.apache.org/r/39583/#comment169255>
Let's move this into `<stout/error.hpp>`
3rdparty/libprocess/3rdparty/stout/include/stout/windows/error.hpp (lines 1 - 13)
<https://reviews.apache.org/r/39583/#comment169256>
new license header format.
3rdparty/libprocess/3rdparty/stout/include/stout/windows/error.hpp (line 21)
<https://reviews.apache.org/r/39583/#comment169257>
Maybe we can drop the `Try` specific part of this comment.
3rdparty/libprocess/3rdparty/stout/include/stout/windows/error.hpp (line 43)
<https://reviews.apache.org/r/39583/#comment169261>
Full words if not part of the API.
3rdparty/libprocess/3rdparty/stout/include/stout/windows/error.hpp (line 47)
<https://reviews.apache.org/r/39583/#comment169260>
new line.
3rdparty/libprocess/3rdparty/stout/include/stout/windows/error.hpp (lines 61 - 63)
<https://reviews.apache.org/r/39583/#comment169262>
let's clarify this as per offline discussion.
3rdparty/libprocess/3rdparty/stout/include/stout/windows/error.hpp (line 77)
<https://reviews.apache.org/r/39583/#comment169264>
You can drop the "as far as I can tell"
3rdparty/libprocess/3rdparty/stout/include/stout/windows/error.hpp (lines 82 - 88)
<https://reviews.apache.org/r/39583/#comment169266>
indentation.
3rdparty/libprocess/3rdparty/stout/include/stout/windows/error.hpp (line 86)
<https://reviews.apache.org/r/39583/#comment169267>
I think we put a space between the old c-style cast and the value. Can you check this?
- Joris Van Remoortere
On Dec. 10, 2015, 2:14 a.m., Alex Clemmer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39583/
> -----------------------------------------------------------
>
> (Updated Dec. 10, 2015, 2:14 a.m.)
>
>
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
>
>
> Bugs: MESOS-4110
> https://issues.apache.org/jira/browse/MESOS-4110
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Windows: Added `WindowsError` to parallel `ErrnoError`.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/include/Makefile.am a8c35c086ecae21701f6a720f25231c1b0d4e329
> 3rdparty/libprocess/3rdparty/stout/include/stout/try.hpp fdd33512c8d8752093f72f597a7d647eb5e3c285
> 3rdparty/libprocess/3rdparty/stout/include/stout/windows/error.hpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/39583/diff/
>
>
> Testing
> -------
>
> `make check` from autotools on Ubuntu 15.
> `make check` from CMake on OS X 10.10.
> Ran `check` project in VS on Windows 10.
>
>
> Thanks,
>
> Alex Clemmer
>
>
Re: Review Request 39583: Windows: Added `WindowsError` to parallel
`ErrnoError`.
Posted by Joris Van Remoortere <jo...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39583/#review110325
-----------------------------------------------------------
Ship it!
Let's use snake_case in stout.
3rdparty/libprocess/3rdparty/stout/include/Makefile.am (line 142)
<https://reviews.apache.org/r/39583/#comment170142>
We're now missing `stout/errorbase.hpp`
- Joris Van Remoortere
On Dec. 12, 2015, 11:08 p.m., Alex Clemmer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39583/
> -----------------------------------------------------------
>
> (Updated Dec. 12, 2015, 11:08 p.m.)
>
>
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
>
>
> Bugs: MESOS-4110
> https://issues.apache.org/jira/browse/MESOS-4110
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Windows: Added `WindowsError` to parallel `ErrnoError`.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/include/Makefile.am a25e2c1e5584e744c666bbc654eafbfc5f7b10e6
> 3rdparty/libprocess/3rdparty/stout/include/stout/error.hpp 605c23bb9273f12dabbd2d6fa87ca0b06d9958ca
> 3rdparty/libprocess/3rdparty/stout/include/stout/errorbase.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/include/stout/try.hpp 9e3b007d9f8db1e16c1f6928d85c230612741cc0
> 3rdparty/libprocess/3rdparty/stout/include/stout/windows/error.hpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/39583/diff/
>
>
> Testing
> -------
>
> `make check` from autotools on Ubuntu 15.
> `make check` from CMake on OS X 10.10.
> Ran `check` project in VS on Windows 10.
>
>
> Thanks,
>
> Alex Clemmer
>
>
Re: Review Request 39583: Windows: Added `WindowsError` to parallel
`ErrnoError`.
Posted by Alex Clemmer <cl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39583/
-----------------------------------------------------------
(Updated Dec. 12, 2015, 11:08 p.m.)
Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
Bugs: MESOS-4110
https://issues.apache.org/jira/browse/MESOS-4110
Repository: mesos
Description
-------
Windows: Added `WindowsError` to parallel `ErrnoError`.
Diffs (updated)
-----
3rdparty/libprocess/3rdparty/stout/include/Makefile.am a25e2c1e5584e744c666bbc654eafbfc5f7b10e6
3rdparty/libprocess/3rdparty/stout/include/stout/error.hpp 605c23bb9273f12dabbd2d6fa87ca0b06d9958ca
3rdparty/libprocess/3rdparty/stout/include/stout/errorbase.hpp PRE-CREATION
3rdparty/libprocess/3rdparty/stout/include/stout/try.hpp 9e3b007d9f8db1e16c1f6928d85c230612741cc0
3rdparty/libprocess/3rdparty/stout/include/stout/windows/error.hpp PRE-CREATION
Diff: https://reviews.apache.org/r/39583/diff/
Testing
-------
`make check` from autotools on Ubuntu 15.
`make check` from CMake on OS X 10.10.
Ran `check` project in VS on Windows 10.
Thanks,
Alex Clemmer
Re: Review Request 39583: Windows: Added `WindowsError` to parallel
`ErrnoError`.
Posted by Alex Clemmer <cl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39583/
-----------------------------------------------------------
(Updated Dec. 10, 2015, 2:14 a.m.)
Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
Bugs: MESOS-4110
https://issues.apache.org/jira/browse/MESOS-4110
Repository: mesos
Description
-------
Windows: Added `WindowsError` to parallel `ErrnoError`.
Diffs
-----
3rdparty/libprocess/3rdparty/stout/include/Makefile.am a8c35c086ecae21701f6a720f25231c1b0d4e329
3rdparty/libprocess/3rdparty/stout/include/stout/try.hpp fdd33512c8d8752093f72f597a7d647eb5e3c285
3rdparty/libprocess/3rdparty/stout/include/stout/windows/error.hpp PRE-CREATION
Diff: https://reviews.apache.org/r/39583/diff/
Testing
-------
`make check` from autotools on Ubuntu 15.
`make check` from CMake on OS X 10.10.
Ran `check` project in VS on Windows 10.
Thanks,
Alex Clemmer
Re: Review Request 39583: Windows: Added `WindowsError` to parallel
`ErrnoError`.
Posted by Alex Clemmer <cl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39583/
-----------------------------------------------------------
(Updated Nov. 16, 2015, 9:13 a.m.)
Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
Repository: mesos
Description
-------
Windows: Added `WindowsError` to parallel `ErrnoError`.
Diffs (updated)
-----
3rdparty/libprocess/3rdparty/stout/include/Makefile.am a8c35c086ecae21701f6a720f25231c1b0d4e329
3rdparty/libprocess/3rdparty/stout/include/stout/try.hpp fdd33512c8d8752093f72f597a7d647eb5e3c285
3rdparty/libprocess/3rdparty/stout/include/stout/windows/error.hpp PRE-CREATION
Diff: https://reviews.apache.org/r/39583/diff/
Testing
-------
`make check` from autotools on Ubuntu 15.
`make check` from CMake on OS X 10.10.
Ran `check` project in VS on Windows 10.
Thanks,
Alex Clemmer
Re: Review Request 39583: Windows: Added `WindowsError` to parallel
`ErrnoError`.
Posted by Joseph Wu <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39583/#review104770
-----------------------------------------------------------
Ship it!
Double-checked build on non-Windows.
- Joseph Wu
On Oct. 29, 2015, 10:55 p.m., Alex Clemmer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39583/
> -----------------------------------------------------------
>
> (Updated Oct. 29, 2015, 10:55 p.m.)
>
>
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Windows: Added `WindowsError` to parallel `ErrnoError`.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/include/Makefile.am 67b142bbc2d80f40a1e893cc5813d58dd2aa8381
> 3rdparty/libprocess/3rdparty/stout/include/stout/try.hpp fdd33512c8d8752093f72f597a7d647eb5e3c285
> 3rdparty/libprocess/3rdparty/stout/include/stout/windows/error.hpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/39583/diff/
>
>
> Testing
> -------
>
> `make check` from autotools on Ubuntu 15.
> `make check` from CMake on OS X 10.10.
> Ran `check` project in VS on Windows 10.
>
>
> Thanks,
>
> Alex Clemmer
>
>
Re: Review Request 39583: Windows: Added `WindowsError` to parallel
`ErrnoError`.
Posted by Alex Clemmer <cl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39583/
-----------------------------------------------------------
(Updated Oct. 30, 2015, 5:55 a.m.)
Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
Repository: mesos
Description
-------
Windows: Added `WindowsError` to parallel `ErrnoError`.
Diffs (updated)
-----
3rdparty/libprocess/3rdparty/stout/include/Makefile.am 67b142bbc2d80f40a1e893cc5813d58dd2aa8381
3rdparty/libprocess/3rdparty/stout/include/stout/try.hpp fdd33512c8d8752093f72f597a7d647eb5e3c285
3rdparty/libprocess/3rdparty/stout/include/stout/windows/error.hpp PRE-CREATION
Diff: https://reviews.apache.org/r/39583/diff/
Testing
-------
`make check` from autotools on Ubuntu 15.
`make check` from CMake on OS X 10.10.
Ran `check` project in VS on Windows 10.
Thanks,
Alex Clemmer
Re: Review Request 39583: Windows: Added `WindowsError` to parallel
`ErrnoError`.
Posted by Alex Clemmer <cl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39583/
-----------------------------------------------------------
(Updated Oct. 27, 2015, 8:17 a.m.)
Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph Wu.
Repository: mesos
Description
-------
Windows: Added `WindowsError` to parallel `ErrnoError`.
Diffs (updated)
-----
3rdparty/libprocess/3rdparty/stout/include/Makefile.am ba2836a72ceee948cb43364e80ada9f132f33d04
3rdparty/libprocess/3rdparty/stout/include/stout/try.hpp fdd33512c8d8752093f72f597a7d647eb5e3c285
3rdparty/libprocess/3rdparty/stout/include/stout/windows/error.hpp PRE-CREATION
Diff: https://reviews.apache.org/r/39583/diff/
Testing
-------
`make check` from autotools on Ubuntu 15.
`make check` from CMake on OS X 10.10.
Ran `check` project in VS on Windows 10.
Thanks,
Alex Clemmer