----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45962/#review157436 -----------------------------------------------------------
Fix it, then Ship it! Looks good! :) src/examples/persistent_volume_framework.cpp (line 86) <https://reviews.apache.org/r/45962/#comment228040> An empty line here before the return. src/examples/persistent_volume_framework.cpp (line 122) <https://reviews.apache.org/r/45962/#comment228173> s/In case/In the case/ src/examples/persistent_volume_framework.cpp (line 124) <https://reviews.apache.org/r/45962/#comment228174> s/in case/in the case/ src/examples/persistent_volume_framework.cpp (line 150) <https://reviews.apache.org/r/45962/#comment228186> As Gaston suggested, embeding sharedness in the name probably makes it more readable in the logs. Maybe `"shared-shard-"`? If we do that, no need to do `i + numShards`? Just do `stringify(i)`? src/examples/persistent_volume_framework.cpp (lines 196 - 198) <https://reviews.apache.org/r/45962/#comment228188> Use `Shard::INIT` as discussed. Then we can probably remove the comment. Consider the state machine already initialized (to INIT). src/examples/persistent_volume_framework.cpp (lines 217 - 218) <https://reviews.apache.org/r/45962/#comment228189> Add a TODO for the custom write command flag here to pair with the TODO below? src/examples/persistent_volume_framework.cpp (line 254) <https://reviews.apache.org/r/45962/#comment228191> Move this empty line to above line 253? src/examples/persistent_volume_framework.cpp (lines 264 - 265) <https://reviews.apache.org/r/45962/#comment228183> Can we briefly explain the reason: the writer task, although launched by the scheduler first, may have yet to write to the disk. src/examples/persistent_volume_framework.cpp (line 267) <https://reviews.apache.org/r/45962/#comment228225> Nit: "specifying a custom" reads better than "specifying a specific"? src/examples/persistent_volume_framework.cpp (lines 269 - 272) <https://reviews.apache.org/r/45962/#comment228185> Can we use ``` R"~( )~" ``` string so we can write the script with with line breaks and indentation as we would usually do? - Jiang Yan Xu On Nov. 28, 2016, 4:25 p.m., Anindya Sinha wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/45962/ > ----------------------------------------------------------- > > (Updated Nov. 28, 2016, 4:25 p.m.) > > > Review request for mesos, Greg Mann, Jie Yu, and Jiang Yan Xu. > > > Bugs: MESOS-4431 > https://issues.apache.org/jira/browse/MESOS-4431 > > > Repository: mesos > > > Description > ------- > > Updated a persistent volume test framework to include shared volumes. > > > Diffs > ----- > > src/examples/persistent_volume_framework.cpp > 9d45bb496c4cf378af429b5aa970bf81450e482a > > Diff: https://reviews.apache.org/r/45962/diff/ > > > Testing > ------- > > New test framework for shared resources added. > Tests successful. > > > Thanks, > > Anindya Sinha > >
