mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Bernd Mathiske" <be...@mesosphere.io>
Subject Re: Review Request 40731: Adding the test framework submitted by Mandeep (@mchadha) https://reviews.apache.org/r/39056/
Date Mon, 30 Nov 2015 14:15:34 GMT

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



src/tests/reservation_tests.cpp (line 168)
<https://reviews.apache.org/r/40731/#comment167714>

    s/double/a double
    
    But the whole sentence is warped. Suggestion:
    
    "This involves a value for CPUs of type double, which triggers a problematic floating
point comparison."



src/tests/reservation_tests.cpp (line 172)
<https://reviews.apache.org/r/40731/#comment167730>

    Please write a comment in the code below that explains where the comparison occurs.



src/tests/reservation_tests.cpp (line 173)
<https://reviews.apache.org/r/40731/#comment167715>

    s/with/with an/



src/tests/reservation_tests.cpp (line 176)
<https://reviews.apache.org/r/40731/#comment167717>

    Also see the indentations in various other statements involving argument passing, etc.
Best to sit down with one of us at the same screen and then clean up all these indentations
in one swoop. Respectively see: 
    http://mesos.apache.org/documentation/latest/c++-style-guide/



src/tests/reservation_tests.cpp (line 198)
<https://reviews.apache.org/r/40731/#comment167718>

    This can be made less ambigous:
    
    "We apply a filter that causes these resources not to be filtered (default would be 5
seconds).



src/tests/reservation_tests.cpp (line 208)
<https://reviews.apache.org/r/40731/#comment167719>

    s/We use this to/
    
    Redundant.



src/tests/reservation_tests.cpp (line 219)
<https://reviews.apache.org/r/40731/#comment167722>

    This comment pertains to line 225, not 220.



src/tests/reservation_tests.cpp (line 227)
<https://reviews.apache.org/r/40731/#comment167725>

    If you use "next" here and below for two different offers, it gets confusing. Better "second",
"third".



src/tests/reservation_tests.cpp (line 235)
<https://reviews.apache.org/r/40731/#comment167723>

    This comment seems to belong to line 241.



src/tests/reservation_tests.cpp (line 239)
<https://reviews.apache.org/r/40731/#comment167727>

    Better to use freah variables like offer 2, offer3. It will be easier to grasp where in
the algorithm we are then.



src/tests/reservation_tests.cpp (line 248)
<https://reviews.apache.org/r/40731/#comment167731>

    s/RESERVE/UNRESERVE/ ?
    
    What is the intention here? Please explain.



src/tests/reservation_tests.cpp (line 251)
<https://reviews.apache.org/r/40731/#comment167724>

    This comment pertains to line 257.
    
    s/reserved/unreserved/ ?


- Bernd Mathiske


On Nov. 30, 2015, 5:39 a.m., Avinash sridharan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40731/
> -----------------------------------------------------------
> 
> (Updated Nov. 30, 2015, 5:39 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Klaus Ma, and Neil Conway.
> 
> 
> Bugs: MESOS-3552
>     https://issues.apache.org/jira/browse/MESOS-3552
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Adding the test framework submitted by Mandeep (@mchadha) https://reviews.apache.org/r/39056/
> 
> 
> Diffs
> -----
> 
>   src/tests/reservation_tests.cpp 15d180f92ec0aea99e6f3a7d0b505c62bd207b61 
> 
> Diff: https://reviews.apache.org/r/40731/diff/
> 
> 
> Testing
> -------
> 
> Ran make check after adding Mandeep's test case to the GTEST framework.
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>


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