mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Joris Van Remoortere <joris.van.remoort...@gmail.com>
Subject Re: Review Request 47169: Windows: Added jobs support.
Date Tue, 10 May 2016 16:31:23 GMT

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




3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/kill.hpp (lines 30 - 51)
<https://reviews.apache.org/r/47169/#comment196660>

    I'm not sure I understand why this review is touching anything in this function.
    Any fixes here should have been in the previous review.



3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/killtree.hpp (lines 49 - 50)
<https://reviews.apache.org/r/47169/#comment196662>

    new line before return statement.
    Brace `}` should be on a separate line after the return statement.



3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/killtree.hpp (line 50)
<https://reviews.apache.org/r/47169/#comment196663>

    Why is returning an empty list here ok? The posix implementation doesn't do that, does
it?
    The comment says it return the process trees that were signaled.



3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp (lines 669 - 670)
<https://reviews.apache.org/r/47169/#comment196666>

    Can we add a comment as to what this does, and what the overall strategy is?



3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp (lines 677 - 678)
<https://reviews.apache.org/r/47169/#comment196669>

    ```
    HANDLE process_handle = ::OpenProcess(
        PROCESS_SET_QUOTA | PROCESS_TERMINATE, 
        false, 
        pid);
    ```



3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp (line 681)
<https://reviews.apache.org/r/47169/#comment196670>

    Is there no extra information we can pass from this failure? No error code?
    Same question for the other errors returned below.



3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp (lines 703 - 704)
<https://reviews.apache.org/r/47169/#comment196671>

    Please add a comment. Add a reference to the overall strategy comment you will add for
`create_job`.



3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp (lines 711 - 712)
<https://reviews.apache.org/r/47169/#comment196672>

    indentation.
    ```
    HANDLE job_handle =
      OpenJobObject(JOB_OBJECT_TERMINATE, FALSE, alpha_pid.get().c_str());
    ```
    If an expression doesn't fit, indent by 2.
    We indent by 4 for function arguments.



3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp (line 722)
<https://reviews.apache.org/r/47169/#comment196674>

    No message?



3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp (line 724)
<https://reviews.apache.org/r/47169/#comment196673>

    new line before return please.


- Joris Van Remoortere


On May 10, 2016, 8:53 a.m., Daniel Pravat wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47169/
> -----------------------------------------------------------
> 
> (Updated May 10, 2016, 8:53 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Alex Clemmer, Joris Van Remoortere, and
Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Windows: Added jobs support.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/kill.hpp PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/killtree.hpp b075d625541ed6c10192e3e98bf399b38b69cdc5

>   3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp 37ef33257830e32875500537df7af38757c6efac

> 
> Diff: https://reviews.apache.org/r/47169/diff/
> 
> 
> Testing
> -------
> 
> Windows: build/test with Marathon
> 
> 
> Thanks,
> 
> Daniel Pravat
> 
>


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