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

Ship it!



3rdparty/libprocess/include/process/metrics/counter.hpp (line 78)
<https://reviews.apache.org/r/37877/#comment154505>

    Since we're touching this, can we use a more meaningful variable name? We 
use full words for variable names in Mesos. (Not yours, I know ;-))
    
    Also pointing out the explicit specialization mentioned in a prior review.



3rdparty/libprocess/include/process/process.hpp (line 335)
<https://reviews.apache.org/r/37877/#comment154503>

    Is this a place where we want to change the code to use a more suitable 
type? My thoughts are "If we're reference counting, we should be unsigned" and 
"If we're on a 64-bit architecture we might as well use a long to avoid chances 
of overflowing the ref count".
    
    Thoughts?
    
    We could do this in a subsequent review to separate the refactorings.



3rdparty/libprocess/src/latch.cpp (lines 41 - 44)
<https://reviews.apache.org/r/37877/#comment154508>

    Please use full words for variable names.
    
    Similar below.



3rdparty/libprocess/src/process.cpp (line 430)
<https://reviews.apache.org/r/37877/#comment154509>

    Another one where I'm curious if it would be worth increasing the range 
(and possibly only expressing valid values, >=0) for safety.



3rdparty/libprocess/src/process.cpp (lines 757 - 773)
<https://reviews.apache.org/r/37877/#comment154521>

    Would you agree that what is happening here is not immediately obvious?
    
    Now that you understand it, would you mind adding a few comments around the 
different exit conditions and which stage of intitialization they represent?
    
    In particular I think the exit condition around 769 could use a comment.



3rdparty/libprocess/src/process.cpp (line 767)
<https://reviews.apache.org/r/37877/#comment154517>

    Can you wrap compare_exchange_strong in backticks ```



3rdparty/libprocess/src/process.cpp (line 768)
<https://reviews.apache.org/r/37877/#comment154516>

    Full variable names.



3rdparty/libprocess/src/process.cpp (line 953)
<https://reviews.apache.org/r/37877/#comment154519>

    let's use backticks ```



3rdparty/libprocess/src/process.cpp (line 1023)
<https://reviews.apache.org/r/37877/#comment154511>

    nice catch.



3rdparty/libprocess/src/process.cpp (line 2826)
<https://reviews.apache.org/r/37877/#comment154512>

    nice catch.


- Joris Van Remoortere


On Sept. 9, 2015, 4:01 p.m., Neil Conway wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37877/
> -----------------------------------------------------------
> 
> (Updated Sept. 9, 2015, 4:01 p.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere and switched to 'mcypark'.
> 
> 
> Bugs: MESOS-3326
>     https://issues.apache.org/jira/browse/MESOS-3326
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> MESOS-3326.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/include/process/latch.hpp 
> a1a2227a9edcc31fd82c6410262aa4565fd66cb2 
>   3rdparty/libprocess/include/process/metrics/counter.hpp 
> e51a8beb80b15dd64aa2e481036ae8ba37125640 
>   3rdparty/libprocess/include/process/process.hpp 
> 009f7c4167fa379ac6b1c267e1b4d5fcdf28d697 
>   3rdparty/libprocess/src/clock.cpp 09c60e5a5d9bd9fc5511e57f3209fad7dbf834d6 
>   3rdparty/libprocess/src/latch.cpp f7d94d92e85c58878d98e13757b6fc37837ca977 
>   3rdparty/libprocess/src/process.cpp 
> 755187c8761137cb2bf2f7295b29a63f63c68bc6 
>   3rdparty/libprocess/src/process_reference.hpp 
> f8df4a6dcf01bb7af750c1ed9e85c64cea2042c5 
> 
> Diff: https://reviews.apache.org/r/37877/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Reply via email to