You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Li Li <li...@hotmail.com> on 2017/07/15 01:07:44 UTC
Review Request 60892: Fixed the hard-coded forward slash which breaks
windows docker container task launching in DC/OS path
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60892/
-----------------------------------------------------------
Review request for mesos, Andrew Schwartzmeyer and Joseph Wu.
Bugs: MESOS-7797
https://issues.apache.org/jira/browse/MESOS-7797
Repository: mesos
Description
-------
Fixed the hard-coded forward slash which breaks windows docker container task launching in DC/OS path
Diffs
-----
src/slave/paths.cpp c08e83c14be30c9ef376326d23a8ec6b6b9ff246
Diff: https://reviews.apache.org/r/60892/diff/1/
Testing
-------
Testing is done.
Thanks,
Li Li
Re: Review Request 60892: Fixed the hard-coded forward slash which
breaks windows docker container task launching in DC/OS path
Posted by Joseph Wu <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60892/#review180624
-----------------------------------------------------------
Ship it!
This is a small fix, so I'm going to point out the things I modified and just make them before committing:
* The commit message is limited to 72 characters, so I'll rephrase it.
* There is a test `PathsTest.ParseExecutorRunPath` in src/tests/paths_tests.cpp which exercises this function. I will un-disable it since it will pass now.
src/slave/paths.cpp
Line 91 (original), 91 (patched)
<https://reviews.apache.org/r/60892/#comment255823>
This line should be broken into 2 lines, to fit under the 80 character line limit.
- Joseph Wu
On July 14, 2017, 6:13 p.m., Li Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60892/
> -----------------------------------------------------------
>
> (Updated July 14, 2017, 6:13 p.m.)
>
>
> Review request for mesos, Andrew Schwartzmeyer and Joseph Wu.
>
>
> Bugs: MESOS-7797
> https://issues.apache.org/jira/browse/MESOS-7797
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Fixed the hard-coded forward slash which breaks windows docker container task launching in DC/OS path
>
>
> Diffs
> -----
>
> src/slave/paths.cpp c08e83c14be30c9ef376326d23a8ec6b6b9ff246
>
>
> Diff: https://reviews.apache.org/r/60892/diff/1/
>
>
> Testing
> -------
>
> Testing Done on Windows system, including all unit tests. Also verify that with the fix the Windows docker container could be launched properly in DC/OS.
>
>
> Thanks,
>
> Li Li
>
>
Re: Review Request 60892: Fixed the hard-coded forward slash which
breaks windows docker container task launching in DC/OS path
Posted by Li Li <li...@hotmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60892/
-----------------------------------------------------------
(Updated July 15, 2017, 1:13 a.m.)
Review request for mesos, Andrew Schwartzmeyer and Joseph Wu.
Bugs: MESOS-7797
https://issues.apache.org/jira/browse/MESOS-7797
Repository: mesos
Description
-------
Fixed the hard-coded forward slash which breaks windows docker container task launching in DC/OS path
Diffs
-----
src/slave/paths.cpp c08e83c14be30c9ef376326d23a8ec6b6b9ff246
Diff: https://reviews.apache.org/r/60892/diff/1/
Testing (updated)
-------
Testing Done on Windows system, including all unit tests. Also verify that with the fix the Windows docker container could be launched properly in DC/OS.
Thanks,
Li Li
Re: Review Request 60892: Fixed the hard-coded forward slash which
breaks windows docker container task launching in DC/OS path
Posted by Andrew Schwartzmeyer <an...@schwartzmeyer.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60892/#review180621
-----------------------------------------------------------
Fix it, then Ship it!
Other than style, this patch looks good.
src/slave/paths.cpp
Line 91 (original), 91 (patched)
<https://reviews.apache.org/r/60892/#comment255820>
This line should be broken at 80 characters per the style guide. But otherwise looks good.
- Andrew Schwartzmeyer
On July 15, 2017, 1:13 a.m., Li Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60892/
> -----------------------------------------------------------
>
> (Updated July 15, 2017, 1:13 a.m.)
>
>
> Review request for mesos, Andrew Schwartzmeyer and Joseph Wu.
>
>
> Bugs: MESOS-7797
> https://issues.apache.org/jira/browse/MESOS-7797
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Fixed the hard-coded forward slash which breaks windows docker container task launching in DC/OS path
>
>
> Diffs
> -----
>
> src/slave/paths.cpp c08e83c14be30c9ef376326d23a8ec6b6b9ff246
>
>
> Diff: https://reviews.apache.org/r/60892/diff/1/
>
>
> Testing
> -------
>
> Testing Done on Windows system, including all unit test. Also verify that with the fix the Windows docker container could be launched properly in DC/OS.
>
>
> Thanks,
>
> Li Li
>
>
Re: Review Request 60892: Fixed the hard-coded forward slash which
breaks windows docker container task launching in DC/OS path
Posted by Li Li <li...@hotmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60892/
-----------------------------------------------------------
(Updated July 15, 2017, 1:13 a.m.)
Review request for mesos, Andrew Schwartzmeyer and Joseph Wu.
Bugs: MESOS-7797
https://issues.apache.org/jira/browse/MESOS-7797
Repository: mesos
Description
-------
Fixed the hard-coded forward slash which breaks windows docker container task launching in DC/OS path
Diffs
-----
src/slave/paths.cpp c08e83c14be30c9ef376326d23a8ec6b6b9ff246
Diff: https://reviews.apache.org/r/60892/diff/1/
Testing (updated)
-------
Testing Done on Windows system, including all unit test. Also verify that with the fix the Windows docker container could be launched properly in DC/OS.
Thanks,
Li Li
Re: Review Request 60892: Fixed the hard-coded forward slash which
breaks windows docker container task launching in DC/OS path
Posted by Li Li <li...@hotmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60892/
-----------------------------------------------------------
(Updated July 15, 2017, 1:11 a.m.)
Review request for mesos, Andrew Schwartzmeyer and Joseph Wu.
Bugs: MESOS-7797
https://issues.apache.org/jira/browse/MESOS-7797
Repository: mesos
Description
-------
Fixed the hard-coded forward slash which breaks windows docker container task launching in DC/OS path
Diffs
-----
src/slave/paths.cpp c08e83c14be30c9ef376326d23a8ec6b6b9ff246
Diff: https://reviews.apache.org/r/60892/diff/1/
Testing (updated)
-------
Testing Done
Thanks,
Li Li