mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Ian Downes" <ian.dow...@gmail.com>
Subject Re: Review Request 34558: Move port mapping isolator configuration settings to file local scope for easier sharing.
Date Fri, 22 May 2015 07:18:03 GMT

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


bunch of grammatical stuff, much of it not actually yours but might as well clean it up while
you're in this file.


src/tests/port_mapping_tests.cpp
<https://reviews.apache.org/r/34558/#comment136309>

    any particular reason why? what about 1/1024/1024?



src/tests/port_mapping_tests.cpp
<https://reviews.apache.org/r/34558/#comment136307>

    Don't include the ';' in these strings.



src/tests/port_mapping_tests.cpp
<https://reviews.apache.org/r/34558/#comment136311>

    Below the start of ip_local_port_range?



src/tests/port_mapping_tests.cpp
<https://reviews.apache.org/r/34558/#comment136312>

    What's the implication of a conflict? Does the test fail on conflict?



src/tests/port_mapping_tests.cpp
<https://reviews.apache.org/r/34558/#comment136308>

    Ditto and elsewhere.



src/tests/port_mapping_tests.cpp
<https://reviews.apache.org/r/34558/#comment136313>

    s/tracking/track/



src/tests/port_mapping_tests.cpp
<https://reviews.apache.org/r/34558/#comment136314>

    s/persistant/persistent/
    specifically, the persistentPorts range defined above.



src/tests/port_mapping_tests.cpp
<https://reviews.apache.org/r/34558/#comment136315>

    s/defined/define/
    s/container 1/container1/



src/tests/port_mapping_tests.cpp
<https://reviews.apache.org/r/34558/#comment136316>

    s/ - /; /



src/tests/port_mapping_tests.cpp
<https://reviews.apache.org/r/34558/#comment136317>

    s/connection/connecting/
    can they even make a connection?



src/tests/port_mapping_tests.cpp
<https://reviews.apache.org/r/34558/#comment136318>

    drop or pull this comment up



src/tests/port_mapping_tests.cpp
<https://reviews.apache.org/r/34558/#comment136319>

    strings::join(";", {...})?



src/tests/port_mapping_tests.cpp
<https://reviews.apache.org/r/34558/#comment136320>

    s/2/two/



src/tests/port_mapping_tests.cpp
<https://reviews.apache.org/r/34558/#comment136321>

    sucessful in that data is written to the appropriate file?



src/tests/port_mapping_tests.cpp
<https://reviews.apache.org/r/34558/#comment136322>

    s/get/receive/



src/tests/port_mapping_tests.cpp
<https://reviews.apache.org/r/34558/#comment136324>

    s/a nc/an nc/ reads better, I think.
    ditto elsewhere


- Ian Downes


On May 21, 2015, 4:31 p.m., Paul Brett wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34558/
> -----------------------------------------------------------
> 
> (Updated May 21, 2015, 4:31 p.m.)
> 
> 
> Review request for mesos, Chi Zhang, Ian Downes, Jie Yu, and Cong Wang.
> 
> 
> Bugs: MESOS-2332
>     https://issues.apache.org/jira/browse/MESOS-2332
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Move port mapping isolator configuration settings to file local scope for easier sharing.
> 
> 
> Diffs
> -----
> 
>   src/tests/port_mapping_tests.cpp b8c2db6d0a02f79d38a21c227575299880980502 
> 
> Diff: https://reviews.apache.org/r/34558/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Paul Brett
> 
>


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