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