----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57762/#review169955 -----------------------------------------------------------
Fix it, then Ship it! I've got some nits, but nothing major. src/launcher/executor.cpp Line 473 (original), 483 (patched) <https://reviews.apache.org/r/57762/#comment242674> Technically the default, so you don't have to set it. src/launcher/executor.cpp Lines 491 (patched) <https://reviews.apache.org/r/57762/#comment242675> Not my favorite wrapping, but I'd allow it. I'd probably prefer: ``` foreach (const Environment::Variable& variable, taskEnvironment->variables()) { ``` src/launcher/executor.cpp Lines 494 (patched) <https://reviews.apache.org/r/57762/#comment242677> "Overwriting environment variable 'foo' with value from task environment." src/launcher/executor.cpp Lines 497 (patched) <https://reviews.apache.org/r/57762/#comment242676> No need to overwrite if the value is the same. Not a big deal if you do though. src/launcher/executor.cpp Lines 507 (patched) <https://reviews.apache.org/r/57762/#comment242678> "Overwriting environment variable 'foo' with value from command environment." - Adam B On March 23, 2017, 7:24 p.m., Till Toenshoff wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/57762/ > ----------------------------------------------------------- > > (Updated March 23, 2017, 7:24 p.m.) > > > Review request for mesos, Adam B, Benjamin Bannier, Greg Mann, Jie Yu, and > Joseph Wu. > > > Bugs: MESOS-7263 > https://issues.apache.org/jira/browse/MESOS-7263 > > > Repository: mesos > > > Description > ------- > > see summary. > > > Diffs > ----- > > src/launcher/executor.cpp db703f054decbca62f7fbe98f5d28f6e4c6c9b0f > > > Diff: https://reviews.apache.org/r/57762/diff/7/ > > > Testing > ------- > > make check + functional testing.. > > > Thanks, > > Till Toenshoff > >
