mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Alex Clemmer" <clemmer.alexan...@gmail.com>
Subject Re: Review Request 37291: Add missing unimplemented.hpp to windows specific OS code.
Date Mon, 10 Aug 2015 18:26:21 GMT

-----------------------------------------------------------
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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message