Re: Review Request 51784: Supported merging the launch command from isolators.

2016-09-30 Thread Benjamin Bannier
> On Sept. 12, 2016, 10:16 p.m., Gilbert Song wrote: > > src/slave/containerizer/mesos/containerizer.cpp, line 1109 > > > > > > Could we just have `arguments` MergeFrom? Because singular field (e.g., > > `value`)

Re: Review Request 51784: Supported merging the launch command from isolators.

2016-09-13 Thread Benjamin Bannier
> On Sept. 12, 2016, 10:16 p.m., Gilbert Song wrote: > > src/slave/containerizer/mesos/containerizer.cpp, line 1109 > > > > > > Could we just have `arguments` MergeFrom? Because singular field (e.g., > > `value`)

Re: Review Request 51784: Supported merging the launch command from isolators.

2016-09-13 Thread Benjamin Bannier
> On Sept. 12, 2016, 3:46 p.m., Benjamin Bannier wrote: > > src/slave/containerizer/mesos/containerizer.cpp, lines 1105-1107 > > > > > > IMHO this is overly simplistic. We should always be prepared for > > modules

Re: Review Request 51784: Supported merging the launch command from isolators.

2016-09-12 Thread Gilbert Song
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51784/#review148571 --- Fix it, then Ship it!

Re: Review Request 51784: Supported merging the launch command from isolators.

2016-09-12 Thread Jie Yu
> On Sept. 12, 2016, 1:46 p.m., Benjamin Bannier wrote: > > I am not really sure it is a good idea to follow this approach. It might > > make things simpler for anybody working on the Mesos containerizer, but I > > fear it might make the interfaces we expose hard to use for everybody else > >

Re: Review Request 51784: Supported merging the launch command from isolators.

2016-09-12 Thread Benjamin Bannier
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51784/#review148482 --- I am not really sure it is a good idea to follow this approach.

Re: Review Request 51784: Supported merging the launch command from isolators.

2016-09-10 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51784/#review148413 --- Patch looks great! Reviews applied: [51784] Passed command: