----------------------------------------------------------- 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 > >
