mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Alexander Rukletsov <ruklet...@gmail.com>
Subject Re: Review Request 61137: Cleaned up style in example frameworks.
Date Thu, 10 Aug 2017 16:27:01 GMT

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


Fix it, then Ship it!




Thanks for the cleanup! I'll fix the issues and commit it for you.


src/examples/balloon_framework.cpp
Lines 261-266 (original), 261-265 (patched)
<https://reviews.apache.org/r/61137/#comment258518>

    I'd say the previous version is more readable. Let's keep it.



src/examples/balloon_framework.cpp
Lines 551-552 (original), 541-542 (patched)
<https://reviews.apache.org/r/61137/#comment258541>

    There is not much value in this change (same amount of lines, no explicit violation of
our style), hence let leave it as is.
    
    Even though I guess clang-format agrees with your suggestion.



src/examples/docker_no_executor_framework.cpp
Lines 95-96 (original), 95-96 (patched)
<https://reviews.apache.org/r/61137/#comment258521>

    There is not much value in this change (same amount of lines, no explicit violation of
our style), hence let leave it as is.



src/examples/dynamic_reservation_framework.cpp
Lines 229-231 (original), 229-230 (patched)
<https://reviews.apache.org/r/61137/#comment258520>

    Let's keep this as is



src/examples/load_generator_framework.cpp
Line 69 (original), 69 (patched)
<https://reviews.apache.org/r/61137/#comment258542>

    Good catch!



src/examples/load_generator_framework.cpp
Lines 72-74 (original), 72-74 (patched)
<https://reviews.apache.org/r/61137/#comment258523>

    There is not much value in this change (same amount of lines, no explicit violation of
our style), hence let leave it as is.



src/examples/long_lived_framework.cpp
Lines 305-308 (original), 305-308 (patched)
<https://reviews.apache.org/r/61137/#comment258524>

    There is not much value in this change (same amount of lines, no explicit violation of
our style), hence let leave it as is.



src/examples/long_lived_framework.cpp
Lines 351-353 (original), 351-353 (patched)
<https://reviews.apache.org/r/61137/#comment258525>

    There is not much value in this change (same amount of lines, no explicit violation of
our style), hence let leave it as is.



src/examples/long_lived_framework.cpp
Lines 636-637 (original), 636-637 (patched)
<https://reviews.apache.org/r/61137/#comment258544>

    There is not much value in this change (same amount of lines, no explicit violation of
our style), hence let leave it as is.



src/examples/no_executor_framework.cpp
Lines 152-153 (original), 147 (patched)
<https://reviews.apache.org/r/61137/#comment258526>

    Let's keep it as is for readability.



src/examples/no_executor_framework.cpp
Lines 177-180 (original), 171-174 (patched)
<https://reviews.apache.org/r/61137/#comment258545>

    There is not much value in this change (same amount of lines, no explicit violation of
our style), hence let leave it as is.



src/examples/no_executor_framework.cpp
Lines 210-211 (original), 202-203 (patched)
<https://reviews.apache.org/r/61137/#comment258546>

    There is not much value in this change (same amount of lines, no explicit violation of
our style), hence let leave it as is.



src/examples/persistent_volume_framework.cpp
Lines 418-419 (original), 413-414 (patched)
<https://reviews.apache.org/r/61137/#comment258547>

    There is not much value in this change (same amount of lines, no explicit violation of
our style), hence let leave it as is.



src/examples/persistent_volume_framework.cpp
Lines 463-464 (original), 455 (patched)
<https://reviews.apache.org/r/61137/#comment258548>

    Missing blank line?



src/examples/test_framework.cpp
Lines 107-108 (original), 107-108 (patched)
<https://reviews.apache.org/r/61137/#comment258549>

    There is not much value in this change (same amount of lines, no explicit violation of
our style), hence let leave it as is.



src/examples/test_framework.cpp
Line 140 (original), 140-141 (patched)
<https://reviews.apache.org/r/61137/#comment258550>

    Let's put them on the same line.



src/examples/test_framework.cpp
Lines 152-154 (original), 153-154 (patched)
<https://reviews.apache.org/r/61137/#comment258552>

    Let's keep them as is for readability.



src/examples/test_framework.cpp
Lines 155-160 (original), 155-158 (patched)
<https://reviews.apache.org/r/61137/#comment258551>

    I would argue that the previous variant is more readable. Let's keep it.



src/examples/test_framework.cpp
Lines 176-177 (patched)
<https://reviews.apache.org/r/61137/#comment258553>

    Same line?



src/examples/test_framework.cpp
Lines 178-179 (original), 179-180 (patched)
<https://reviews.apache.org/r/61137/#comment258554>

    Blanke line?



src/examples/test_framework.cpp
Lines 185-186 (patched)
<https://reviews.apache.org/r/61137/#comment258555>

    Put them onto the same line?



src/examples/test_http_framework.cpp
Lines 40-42 (original), 40-41 (patched)
<https://reviews.apache.org/r/61137/#comment258556>

    Remove the duplicate while you're touching these lines.



src/examples/test_http_framework.cpp
Lines 167-168 (original), 167-168 (patched)
<https://reviews.apache.org/r/61137/#comment258557>

    Let's leave as is.



src/examples/test_http_framework.cpp
Lines 171-172 (original), 171-172 (patched)
<https://reviews.apache.org/r/61137/#comment258558>

    Let's leave as is.



src/examples/test_http_framework.cpp
Lines 189-190 (original), 189-191 (patched)
<https://reviews.apache.org/r/61137/#comment258559>

    Let's leave as is.



src/examples/test_http_framework.cpp
Lines 243-244 (original), 244-245 (patched)
<https://reviews.apache.org/r/61137/#comment258560>

    Ditto.



src/examples/test_http_framework.cpp
Lines 319-327 (original), 319-325 (patched)
<https://reviews.apache.org/r/61137/#comment258561>

    Let's leave as is for readability.


- Alexander Rukletsov


On Aug. 7, 2017, 1:53 p.m., Armand Grillet wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61137/
> -----------------------------------------------------------
> 
> (Updated Aug. 7, 2017, 1:53 p.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-7814
>     https://issues.apache.org/jira/browse/MESOS-7814
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/examples/balloon_framework.cpp dfd049b860345adc33c3e774dcdff0320da107f6 
>   src/examples/disk_full_framework.cpp 215b6d29cf5f52973f2b211e9fc72ca6ec2afa7a 
>   src/examples/docker_no_executor_framework.cpp 4a58f11fc8892f23ade1d8c872ab9b4fc580d478

>   src/examples/dynamic_reservation_framework.cpp f3b1c8c4d2684e827fc10776fef4f2287e315b85

>   src/examples/load_generator_framework.cpp abb70f42a3a755afaa1d56b1491058b90958f030

>   src/examples/long_lived_framework.cpp 2a79dfd3f81e5254922895af45c2b72d80ce5f49 
>   src/examples/no_executor_framework.cpp 7d841c6f364e1b671ec829aa9bff3b1f8ecf55ef 
>   src/examples/persistent_volume_framework.cpp 17140294a1eaeeeb92e9e14fc8638182b3a0682e

>   src/examples/test_framework.cpp 9dbc18b039ee13cc1ec9454bd41cb3cfe30d63b4 
>   src/examples/test_http_framework.cpp a0cd84ab6dfdd8d6ed3403c6e06fabae6d5b46c9 
> 
> 
> Diff: https://reviews.apache.org/r/61137/diff/4/
> 
> 
> Testing
> -------
> 
> `$ make check`
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>


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