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


First, let me say thanks for putting this work in, and apologies for not 
communicating the exact status of the port ahead of time -- we're in a rush to 
get things in shape for MesosCon, and a lot of things are in the air right now.

To the matter at hand: I'd actually like to keep the `UNIMPLEMENTED` symbol 
undefined for now, for a few reasons. Let me list them and then propose an 
alternative solution to what I think the problem you're addressing really is. 
Let me preface this by saying I don't see `unimplemented.hpp` in here anywhere, 
so I've made assumptions about what it does. Let me know if they're incorrect.

1. I personally don't want Stout to compile yet. I want it to be very clear to 
people who compile on Windows what works and what doesn't. What I don't want is 
for people to compile the code, think that it works, and then go to run it, and 
have it explode at runtime. It seems better to have it be undefined, which 
makes it very clear at the outset what is supported and what doesn't.
2. In the case that `unimplemented.hpp` _does_ explode at compile time, it's 
not clear to me that there is an advantage to sourcing this to an hpp file. The 
advantage I see is that we might be able to define `UNIMPLEMENTED` to explode 
at runtime or compile time, but I'd rather put this patch in when there is a 
clear need for it.
3. Relatedly, our development model currently is: when we run into an 
`UNIMPLEMENTED` because we are trying to (_e.g._) get the containerizer tests 
to run, it forces us to write code and comments documenting the design 
decisions of the port.

Now, a solution. I think the real problem is that our CMakeLists file for Stout 
tests do not clearly delineate which tests are _supposed_ to compile. What we 
could do instead is simply check in the current CMakeLists, which should make 
it very clear what we have so far.

What do you think of that option?

- Alex Clemmer


On Aug. 10, 2015, 10:17 a.m., haosdent huang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37291/
> -----------------------------------------------------------
> 
> (Updated Aug. 10, 2015, 10:17 a.m.)
> 
> 
> Review request for mesos, Alex Clemmer, Joseph Wu, and Michael Park.
> 
> 
> Bugs: MESOS-3247
>     https://issues.apache.org/jira/browse/MESOS-3247
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Add missing unimplemented.hpp to windows specific OS code.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/close.hpp 
> 134a26b1ff1d063cd2ef18f830378c1e1140ff53 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/exists.hpp 
> 46f8fd2f318e67f93efc5486993aef70daeb71bd 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/fcntl.hpp 
> 0bad61561ae25adb7ff6d731452577133688f8d4 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/fork.hpp 
> c32e7acfab50608e84e6b554a6acd65bc888e40d 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/killtree.hpp 
> ec645cab1b2990cf297b8eb0ca346449cd4748ff 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/ls.hpp 
> 5b6fba13ce215af5801fd0867f6e774e100689ca 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/open.hpp 
> 14fa11765c222cb4e80f5e45360d0f05facb578e 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/permissions.hpp 
> daed4b4e9c76d6e7c043a1fa3a46ab1f3db95f48 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/process.hpp 
> e8238383c3c5feb688b10e37e544556ba9d43107 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/pstree.hpp 
> f75a77fc58db09fadf80409f506852e48a7df7c4 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/read.hpp 
> 09d63329f16f13d408742f9fc8f596d76c4d70c9 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/sendfile.hpp 
> 9658bb8c3cd6d788f970d875a6ed274f5f5065c9 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/shell.hpp 
> 4b7a7bafe1c64183d021b39cf12523250491f0ee 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/signals.hpp 
> 8361a13907ec8044d52545088a767e337dbb1b37 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/stat.hpp 
> 675b2e712358a55b3580026936890eaf80e5af71 
>   3rdparty/libprocess/3rdparty/stout/include/stout/windows/gzip.hpp 
> 017cfb336ae651c150ca35468d5c19838398b419 
>   3rdparty/libprocess/3rdparty/stout/include/stout/windows/net.hpp 
> 4f82796c2b9ef6a9198be145d969c5fce933be49 
>   3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp 
> 2b0966889af73238a08e29f1136d0ce286a0ffda 
> 
> Diff: https://reviews.apache.org/r/37291/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Reply via email to