----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55239/#review161409 -----------------------------------------------------------
Fix it, then Ship it! src/launcher/fetcher.cpp (line 51) <https://reviews.apache.org/r/55239/#comment232650> #include <vector>? src/launcher/fetcher.cpp (line 87) <https://reviews.apache.org/r/55239/#comment232651> This is fine too but apparently the input doesn't need to come from stdin, it can just be an argument. src/launcher/fetcher.cpp (line 96) <https://reviews.apache.org/r/55239/#comment232655> This is me nit-picking but as a general good practice, put a `CHECK_GT(command.size(), 0u)` above so it's future proof (that `command` won't go through some branch that doesn't set it)? src/launcher/fetcher.cpp (line 105) <https://reviews.apache.org/r/55239/#comment232653> Quote the command as well? src/launcher/fetcher.cpp (line 109) <https://reviews.apache.org/r/55239/#comment232652> Just add a simple comment: ``` // `status()` never fails or gets discarded. ``` because this is not obvious. I'll send a patch to better document this. src/launcher/fetcher.cpp (line 113) <https://reviews.apache.org/r/55239/#comment232654> Quote the command as well? - Jiang Yan Xu On Jan. 11, 2017, 4:27 p.m., James Peach wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/55239/ > ----------------------------------------------------------- > > (Updated Jan. 11, 2017, 4:27 p.m.) > > > Review request for mesos, Jie Yu and Jiang Yan Xu. > > > Bugs: MESOS-6862 > https://issues.apache.org/jira/browse/MESOS-6862 > > > Repository: mesos > > > Description > ------- > > Stop using os::system to extract fetcher archives. > > > Diffs > ----- > > src/launcher/fetcher.cpp 4456c28139966e42859cc6d2c79a1d90e83fb22f > > Diff: https://reviews.apache.org/r/55239/diff/ > > > Testing > ------- > > `sudo make check` (Fedora 25) > > > Thanks, > > James Peach > >
