----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60764/#review183435 ----------------------------------------------------------- Fix it, then Ship it! Thanks for the cleanup! src/slave/containerizer/mesos/containerizer.cpp Lines 148 (patched) Let's just inline this method. There is only one user for this method. src/slave/containerizer/mesos/containerizer.cpp Line 154 (original), 169 (patched) Can you create a LinkedHashset (similar to LinkedHashmap) in stout which preserve the insertion order. this solves one issues about the ordering of the isolators is not preserved. I would at least add a TODO here (i do wish you can follow up :)) src/slave/containerizer/mesos/containerizer.cpp Lines 182-184 (original), 203-208 (patched) The indentation should be like the following: ``` switch (std::count_if( isolations->begin(), isolations->end(), [](...) { })) { } ``` similar to what we did for other statements: ``` int foo = func( param1, param2); ``` - Jie Yu On July 11, 2017, 11:13 a.m., James Peach wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/60764/ > ----------------------------------------------------------- > > (Updated July 11, 2017, 11:13 a.m.) > > > Review request for mesos, Qian Zhang and Jiang Yan Xu. > > > Bugs: MESOS-7675 > https://issues.apache.org/jira/browse/MESOS-7675 > > > Repository: mesos > > > Description > ------- > > Refactored the isolator dependency checks to immediately tokenize > the isolator string, which makes it easier to check various consistency > conditions. > > > Diffs > ----- > > src/slave/containerizer/mesos/containerizer.cpp ff192bb085f3554ad1b1f20fb73bf827bf04ef13 > > > Diff: https://reviews.apache.org/r/60764/diff/7/ > > > Testing > ------- > > make check (Fedora 26) > > > Thanks, > > James Peach > >