You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Armand Grillet <ag...@mesosphere.io> on 2017/07/26 14:06:11 UTC

Review Request 61137: Cleaned up style in example frameworks.

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

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 b8c8e387bd8592bc1d27ff62fa9fd5397ad71fb9 
  src/examples/disk_full_framework.cpp 215b6d29cf5f52973f2b211e9fc72ca6ec2afa7a 
  src/examples/docker_no_executor_framework.cpp 4a58f11fc8892f23ade1d8c872ab9b4fc580d478 


Diff: https://reviews.apache.org/r/61137/diff/1/


Testing
-------

`$ make check`


Thanks,

Armand Grillet


Re: Review Request 61137: Cleaned up style in example frameworks.

Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61137/#review181455
-----------------------------------------------------------



Patch looks great!

Reviews applied: [61110, 61111, 61112, 61134, 61135, 61136, 61137]

Passed command: support\windows-build.bat

- Mesos Reviewbot Windows


On July 26, 2017, 2:06 p.m., Armand Grillet wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61137/
> -----------------------------------------------------------
> 
> (Updated July 26, 2017, 2:06 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 b8c8e387bd8592bc1d27ff62fa9fd5397ad71fb9 
>   src/examples/disk_full_framework.cpp 215b6d29cf5f52973f2b211e9fc72ca6ec2afa7a 
>   src/examples/docker_no_executor_framework.cpp 4a58f11fc8892f23ade1d8c872ab9b4fc580d478 
> 
> 
> Diff: https://reviews.apache.org/r/61137/diff/1/
> 
> 
> Testing
> -------
> 
> `$ make check`
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>


Re: Review Request 61137: Cleaned up style in example frameworks.

Posted by Benno Evers <be...@yandex-team.ru>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61137/#review182475
-----------------------------------------------------------


Ship it!




- Benno Evers


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
> 
>


Re: Review Request 61137: Cleaned up style in example frameworks.

Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61137/#review182312
-----------------------------------------------------------



Patch looks great!

Reviews applied: [61112, 61110, 61111, 61137]

Passed command: support\windows-build.bat

- Mesos Reviewbot Windows


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
> 
>


Re: Review Request 61137: Cleaned up style in example frameworks.

Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
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
> 
>


Re: Review Request 61137: Cleaned up style in example frameworks.

Posted by Armand Grillet <ag...@mesosphere.io>.
-----------------------------------------------------------
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.


Changes
-------

Applied changes on every example frameworks.


Bugs: MESOS-7814
    https://issues.apache.org/jira/browse/MESOS-7814


Repository: mesos


Description
-------

See summary.


Diffs (updated)
-----

  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/

Changes: https://reviews.apache.org/r/61137/diff/3-4/


Testing
-------

`$ make check`


Thanks,

Armand Grillet


Re: Review Request 61137: Cleaned up style in example frameworks.

Posted by Armand Grillet <ag...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61137/
-----------------------------------------------------------

(Updated Aug. 1, 2017, 10:21 a.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 b8c8e387bd8592bc1d27ff62fa9fd5397ad71fb9 
  src/examples/disk_full_framework.cpp 215b6d29cf5f52973f2b211e9fc72ca6ec2afa7a 
  src/examples/docker_no_executor_framework.cpp 4a58f11fc8892f23ade1d8c872ab9b4fc580d478 


Diff: https://reviews.apache.org/r/61137/diff/2/


Testing
-------

`$ make check`


Thanks,

Armand Grillet