-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38141/#review98097
-----------------------------------------------------------

Ship it!


LGTM!

Could you run `sudo make check GTEST_FILTER="*Provisioner*"` to make sure Appc 
tests pass?

I originally consolidated all path manipulations into a single paths.hpp|cpp 
but now with provisioner dir part pulled out it may make sense to put the store 
specific ones into the store itself, but that's for another patch I will take 
on myself.


src/slave/containerizer/provisioners/appc/paths.cpp (lines 21 - 23)
<https://reviews.apache.org/r/38141/#comment154347>

    No longer needed.



src/slave/containerizer/provisioners/appc/paths.cpp (line 25)
<https://reviews.apache.org/r/38141/#comment154349>

    Ditto.



src/slave/containerizer/provisioners/appc/paths.cpp (line 28)
<https://reviews.apache.org/r/38141/#comment154350>

    Ditto.



src/slave/containerizer/provisioners/appc/paths.cpp (line 31)
<https://reviews.apache.org/r/38141/#comment154352>

    Ditto.



src/slave/containerizer/provisioners/appc/paths.cpp (line 87)
<https://reviews.apache.org/r/38141/#comment154353>

    A single blank line.



src/slave/containerizer/provisioners/paths.hpp (lines 46 - 48)
<https://reviews.apache.org/r/38141/#comment154343>

    This comment was assuming this was for APPC. Now that it's generic, it 
should be something like:
    
    ```
    // NOTE: Each container could have multiple image types, therefore there
    // can be the same <container_id> directories under each provisioner e.g.,
    // <work_dir>/provisioners/DOCKER, <work_dir>/provisioners/APPC, etc. 
    ```



src/slave/containerizer/provisioners/paths.hpp (lines 49 - 51)
<https://reviews.apache.org/r/38141/#comment154346>

    There is a "For appc" here, can the same be true for docker? I think it 
could.


- Jiang Yan Xu


On Sept. 7, 2015, 2:31 p.m., Timothy Chen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38141/
> -----------------------------------------------------------
> 
> (Updated Sept. 7, 2015, 2:31 p.m.)
> 
> 
> Review request for mesos and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Refactor shared paths in provisioners.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am 5fdca0f574e7e08c4b1aebed0fac39140c19adfe 
>   src/slave/containerizer/provisioners/appc.cpp 
> fc5ee19df51a3543aaf01c2301b976700610ff57 
>   src/slave/containerizer/provisioners/appc/paths.hpp 
> fb3a1a7295809d745dd15bee6db1f7e8dd99ab33 
>   src/slave/containerizer/provisioners/appc/paths.cpp 
> e6be851e24886d7c886adad4c7ea29ded17bdcbe 
>   src/slave/containerizer/provisioners/paths.hpp PRE-CREATION 
>   src/slave/containerizer/provisioners/paths.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38141/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Reply via email to