You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Timothy Chen <tn...@apache.org> on 2015/12/04 10:26:14 UTC
Re: Review Request 40211: Add mesos provisioner doc.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40211/
-----------------------------------------------------------
(Updated Dec. 4, 2015, 9:26 a.m.)
Review request for mesos, Jie Yu and Jojy Varghese.
Repository: mesos
Description
-------
Add mesos provisioner doc.
Diffs (updated)
-----
docs/mesos-provisioner.md PRE-CREATION
Diff: https://reviews.apache.org/r/40211/diff/
Testing
-------
Thanks,
Timothy Chen
Re: Review Request 40211: Add mesos provisioner doc.
Posted by Timothy Chen <tn...@apache.org>.
> On Dec. 9, 2015, 1:42 a.m., Jie Yu wrote:
> > docs/mesos-provisioner.md, line 51
> > <https://reviews.apache.org/r/40211/diff/3/?file=1153688#file1153688line51>
> >
> > Why this limitation. I think we can still use image-in-volume even if command line executor is used, right?
I think it's just current code doesn't support it, but not a technical limitation. I'll remove this.
- Timothy
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40211/#review109432
-----------------------------------------------------------
On Dec. 4, 2015, 9:26 a.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40211/
> -----------------------------------------------------------
>
> (Updated Dec. 4, 2015, 9:26 a.m.)
>
>
> Review request for mesos, Jie Yu and Jojy Varghese.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Add mesos provisioner doc.
>
>
> Diffs
> -----
>
> docs/mesos-provisioner.md PRE-CREATION
>
> Diff: https://reviews.apache.org/r/40211/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Timothy Chen
>
>
Re: Review Request 40211: Add mesos provisioner doc.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40211/#review109432
-----------------------------------------------------------
Ship it!
Looks good overall. Thanks Tim!
docs/mesos-provisioner.md (line 5)
<https://reviews.apache.org/r/40211/#comment168957>
It's not just container filesystem spec, right? It also includes runtime specification. Can you just say:
```
There are multiple container specifications, ...
```
For runC, it's only for runtime right? So your next sentense does not sound so right. How about this:
```
Most of the container specifications, to varying degrees, conflate image format specification with other components of a container, including...
```
docs/mesos-provisioner.md (line 7)
<https://reviews.apache.org/r/40211/#comment168955>
s/R/r/
s/different formats/different image formats/
docs/mesos-provisioner.md (line 13)
<https://reviews.apache.org/r/40211/#comment168961>
I wouldn't mention the runtime configuration here since it's pretty confusing. I would rather add a section later to explain the runtime configuration part (but not in this review probably).
docs/mesos-provisioner.md (line 51)
<https://reviews.apache.org/r/40211/#comment168962>
Why this limitation. I think we can still use image-in-volume even if command line executor is used, right?
docs/mesos-provisioner.md (line 59)
<https://reviews.apache.org/r/40211/#comment168963>
you probably want to explain the backend flag (or add a TODO) since it's not quite intuitive what that is.
docs/mesos-provisioner.md (line 63)
<https://reviews.apache.org/r/40211/#comment168965>
Kill extra blank line
docs/mesos-provisioner.md (line 67)
<https://reviews.apache.org/r/40211/#comment168964>
kill extra blank line
- Jie Yu
On Dec. 4, 2015, 9:26 a.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40211/
> -----------------------------------------------------------
>
> (Updated Dec. 4, 2015, 9:26 a.m.)
>
>
> Review request for mesos, Jie Yu and Jojy Varghese.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Add mesos provisioner doc.
>
>
> Diffs
> -----
>
> docs/mesos-provisioner.md PRE-CREATION
>
> Diff: https://reviews.apache.org/r/40211/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Timothy Chen
>
>
Re: Review Request 40211: Add mesos provisioner doc.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40211/#review108961
-----------------------------------------------------------
Patch looks great!
Reviews applied: [40211]
Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export COMPILER=gcc; ./support/docker_build.sh
- Mesos ReviewBot
On Dec. 4, 2015, 9:26 a.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40211/
> -----------------------------------------------------------
>
> (Updated Dec. 4, 2015, 9:26 a.m.)
>
>
> Review request for mesos, Jie Yu and Jojy Varghese.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Add mesos provisioner doc.
>
>
> Diffs
> -----
>
> docs/mesos-provisioner.md PRE-CREATION
>
> Diff: https://reviews.apache.org/r/40211/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Timothy Chen
>
>