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


Fix it, then Ship it!




Thanks a lot for the cleanup, Gastón!


src/checks/checker.cpp
Line 106 (original), 106 (patched)
<https://reviews.apache.org/r/57854/#comment243408>

    We probably don't need to escape `pid`s.



src/checks/checker.cpp
Line 111 (original), 111 (patched)
<https://reviews.apache.org/r/57854/#comment243409>

    We probably don't need to escape `pid`s.



src/checks/checker.cpp
Line 417 (original), 417 (patched)
<https://reviews.apache.org/r/57854/#comment243418>

    I saw you removed "; aborting" in health checker. Did you also intend to 
remove it in checker?



src/checks/health_checker.cpp
Line 120 (original), 120 (patched)
<https://reviews.apache.org/r/57854/#comment243410>

    We probably don't need to escape `pid`s.



src/checks/health_checker.cpp
Line 125 (original), 125 (patched)
<https://reviews.apache.org/r/57854/#comment243411>

    We probably don't need to escape `pid`s.



src/checks/health_checker.cpp
Lines 288 (patched)
<https://reviews.apache.org/r/57854/#comment243413>

    We usually use semicolons for error concatenation / explanation.



src/checks/health_checker.cpp
Lines 385 (patched)
<https://reviews.apache.org/r/57854/#comment243414>

    Probably semicolon is better here.



src/checks/health_checker.cpp
Lines 475-476 (original), 487-488 (patched)
<https://reviews.apache.org/r/57854/#comment243415>

    Fits one line now.



src/checks/health_checker.cpp
Lines 528-533 (original), 540-551 (patched)
<https://reviews.apache.org/r/57854/#comment243416>

    I don't thin we win a lot by introducing these aliases, because the second 
lambda has to use `this` anyway.



src/checks/health_checker.cpp
Line 946 (original), 961-962 (patched)
<https://reviews.apache.org/r/57854/#comment243419>

    Not sure we should escape port.



src/checks/health_checker.cpp
Lines 996-997 (original), 1017-1018 (patched)
<https://reviews.apache.org/r/57854/#comment243420>

    Fits one line.


- Alexander Rukletsov


On March 29, 2017, 4:37 p.m., Gastón Kleiman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57854/
> -----------------------------------------------------------
> 
> (Updated March 29, 2017, 4:37 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Anand Mazumdar, haosdent 
> huang, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> - Made log/failure messages consistent across both libraries.
> - Task and container IDs are user generated and can contain spaces, so
>   we have to wrap them in single quotes.
> - Removed the redundant task IDs from 'Failure' messages.
> 
> 
> Diffs
> -----
> 
>   src/checks/checker.cpp 314354cc374b453ec12e25e3d4730a92697468cf 
>   src/checks/health_checker.cpp 2211228f7aa0228af64d8fce6c5f2dd1847328f9 
>   src/tests/check_tests.cpp 16f1c07e109e24d475ad593ef1992dfb9f482ba6 
> 
> 
> Diff: https://reviews.apache.org/r/57854/diff/8/
> 
> 
> Testing
> -------
> 
> Did some manual testing and looked at the new log messages, they're so much 
> nicer now <3.
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Reply via email to