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 61495: Add documentation for possible task reasons.
Date Thu, 31 Aug 2017 14:08:44 GMT

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


Fix it, then Ship it!




Looks very good to me. Please have a native speaker look through the write-up before we ship
it: I'm not sure whether appropriate tenses are used in some descriptions. Also please wrap
at 80 chars where possible.


docs/task-state-reasons.md
Lines 18 (patched)
<https://reviews.apache.org/r/61495/#comment260345>

    I'd rather say "authors" or "writers".



docs/task-state-reasons.md
Lines 20 (patched)
<https://reviews.apache.org/r/61495/#comment260348>

    s/which/that



docs/task-state-reasons.md
Lines 25 (patched)
<https://reviews.apache.org/r/61495/#comment260349>

    remove one "the"



docs/task-state-reasons.md
Lines 47-52 (patched)
<https://reviews.apache.org/r/61495/#comment260351>

    Please add a note saying that default executors currently additionally send all unhealthy
updates.



docs/task-state-reasons.md
Lines 80-82 (patched)
<https://reviews.apache.org/r/61495/#comment260352>

    "As of Mesos 1.4 ..." instead of "currently"?



docs/task-state-reasons.md
Lines 95 (patched)
<https://reviews.apache.org/r/61495/#comment260371>

    Wrap `FrameworkInfo` in backticks?



docs/task-state-reasons.md
Lines 98 (patched)
<https://reviews.apache.org/r/61495/#comment260397>

    ... / `SOURCE_AGENT`



docs/task-state-reasons.md
Lines 119 (patched)
<https://reviews.apache.org/r/61495/#comment260372>

    s/memory/disk



docs/task-state-reasons.md
Lines 134 (patched)
<https://reviews.apache.org/r/61495/#comment260373>

    s/re-register/reregister?



docs/task-state-reasons.md
Lines 162-163 (patched)
<https://reviews.apache.org/r/61495/#comment260374>

    You use 3rd and 4th caption level inconsistently: sometimes you repeat caption for the
state, and sometimes you do not.
    
    I would suggest to get rid of the 4th level altogether and have 3rd level captions in
form "For `TASK_KILLED` updates with `SOURCE_SLAVE`"



docs/task-state-reasons.md
Lines 202 (patched)
<https://reviews.apache.org/r/61495/#comment260375>

    s/slave/agent



docs/task-state-reasons.md
Lines 207 (patched)
<https://reviews.apache.org/r/61495/#comment260376>

    s/slave/agent



docs/task-state-reasons.md
Lines 212 (patched)
<https://reviews.apache.org/r/61495/#comment260377>

    s/slave/agent



docs/task-state-reasons.md
Lines 224 (patched)
<https://reviews.apache.org/r/61495/#comment260378>

    s/slave/agent



docs/task-state-reasons.md
Lines 226 (patched)
<https://reviews.apache.org/r/61495/#comment260379>

    s/slave/agent



docs/task-state-reasons.md
Lines 230 (patched)
<https://reviews.apache.org/r/61495/#comment260387>

    Either remove "to" or add "change". Here and below.



docs/task-state-reasons.md
Lines 234 (patched)
<https://reviews.apache.org/r/61495/#comment260385>

    Is this state for v0 API only? If so, please make a note about it.



docs/task-state-reasons.md
Lines 238 (patched)
<https://reviews.apache.org/r/61495/#comment260395>

    Ditto



docs/task-state-reasons.md
Lines 248 (patched)
<https://reviews.apache.org/r/61495/#comment260380>

    s/slave/agent



docs/task-state-reasons.md
Lines 250 (patched)
<https://reviews.apache.org/r/61495/#comment260381>

    s/slave/agent



docs/task-state-reasons.md
Lines 254 (patched)
<https://reviews.apache.org/r/61495/#comment260394>

    Ditto



docs/task-state-reasons.md
Lines 256 (patched)
<https://reviews.apache.org/r/61495/#comment260382>

    s/slave/agent



docs/task-state-reasons.md
Lines 259 (patched)
<https://reviews.apache.org/r/61495/#comment260393>

    Ditto



docs/task-state-reasons.md
Lines 267 (patched)
<https://reviews.apache.org/r/61495/#comment260386>

    remove "to"?



docs/task-state-reasons.md
Lines 273 (patched)
<https://reviews.apache.org/r/61495/#comment260383>

    s/slave/agent



docs/task-state-reasons.md
Lines 282 (patched)
<https://reviews.apache.org/r/61495/#comment260392>

    Ditto



docs/task-state-reasons.md
Lines 291 (patched)
<https://reviews.apache.org/r/61495/#comment260391>

    Ditto



docs/task-state-reasons.md
Lines 305 (patched)
<https://reviews.apache.org/r/61495/#comment260384>

    s/slave/agent



docs/task-state-reasons.md
Lines 309 (patched)
<https://reviews.apache.org/r/61495/#comment260388>

    Ditto



docs/task-state-reasons.md
Lines 318 (patched)
<https://reviews.apache.org/r/61495/#comment260389>

    Ditto



docs/task-state-reasons.md
Lines 326 (patched)
<https://reviews.apache.org/r/61495/#comment260390>

    Ditto



docs/task-state-reasons.md
Lines 413 (patched)
<https://reviews.apache.org/r/61495/#comment260396>

    "originally sent", probably? Instead, you can say "the original ones"



include/mesos/mesos.proto
Lines 2149-2150 (original)
<https://reviews.apache.org/r/61495/#comment260346>

    I thought you remove the comment in the next patch?



include/mesos/v1/mesos.proto
Lines 2132-2133 (original)
<https://reviews.apache.org/r/61495/#comment260347>

    Ditto.


- Alexander Rukletsov


On Aug. 25, 2017, 1:35 p.m., Benno Evers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61495/
> -----------------------------------------------------------
> 
> (Updated Aug. 25, 2017, 1:35 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, James Peach, and Till Toenshoff.
> 
> 
> Bugs: MESOS-5078
>     https://issues.apache.org/jira/browse/MESOS-5078
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Add documentation for possible task reasons.
> 
> 
> Diffs
> -----
> 
>   docs/home.md ad91f2fe23b39d59dc021428899069531fce9970 
>   docs/task-state-reasons.md PRE-CREATION 
>   include/mesos/mesos.proto 86936cf9ad9406fa4c3358ccb3c58e2f3259d65f 
>   include/mesos/v1/mesos.proto 20d9ecff68465c6517a5a239a4bd0f42f82d0cc6 
>   src/master/master.cpp a9d2191d92d0a56c8a137cb28f41a3632db54fe9 
> 
> 
> Diff: https://reviews.apache.org/r/61495/diff/6/
> 
> 
> Testing
> -------
> 
> Built website with site/build.sh and verified it renders ok.
> 
> HTML preview: http://htmlpreview.github.io/?https://github.com/lava/mesos/blob/bennoe/task-reasons/site/publish/documentation/latest/task-state-reasons/index.html
> 
> 
> Thanks,
> 
> Benno Evers
> 
>


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