----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34139/#review90830 -----------------------------------------------------------
src/slave/containerizer/provisioners/appc/discovery.hpp (lines 43 - 46) <https://reviews.apache.org/r/34139/#comment143985> Some comments explaining that the return value is a URI string would be nice. src/slave/containerizer/provisioners/appc/discovery.hpp (line 46) <https://reviews.apache.org/r/34139/#comment143994> The `id` is not use in this review and not specified in https://github.com/appc/spec/blob/master/spec/discovery.md I assume its the sha512 but is it used during discovery? src/slave/containerizer/provisioners/appc/discovery.hpp (line 79) <https://reviews.apache.org/r/34139/#comment144003> No need for the trailing `;` src/slave/containerizer/provisioners/appc/discovery.cpp (line 21) <https://reviews.apache.org/r/34139/#comment143999> Kill empty line. src/slave/containerizer/provisioners/appc/discovery.cpp (line 60) <https://reviews.apache.org/r/34139/#comment143977> canonicalize() not introduced until /r/34142/. Is there anything else outside discovery that uses the method? Can it be moved to class `Discovery` (so we don't have to deal with cyclic review dependency)? src/slave/containerizer/provisioners/appc/discovery.cpp (line 90) <https://reviews.apache.org/r/34139/#comment143998> FWIW https://github.com/appc/spec/blob/master/spec/discovery.md uses https exclusively. src/slave/flags.cpp (line 63) <https://reviews.apache.org/r/34139/#comment144000> List all supported values? src/slave/flags.cpp (line 68) <https://reviews.apache.org/r/34139/#comment144001> State that this is only for local discovery? The sentences already mentions 'local images' but I think --appc_discovery=local is more explict in telling what the operator should do. - Jiang Yan Xu On July 7, 2015, 12:42 p.m., Ian Downes wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/34139/ > ----------------------------------------------------------- > > (Updated July 7, 2015, 12:42 p.m.) > > > Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone. > > > Repository: mesos > > > Description > ------- > > Local and simple discovery only. > > > Diffs > ----- > > src/Makefile.am e7de0f3d1a5efeaef47d5074defe3b40db94f573 > src/slave/containerizer/provisioners/appc/discovery.hpp PRE-CREATION > src/slave/containerizer/provisioners/appc/discovery.cpp PRE-CREATION > src/slave/flags.hpp 7634e368c72e83932dcd992d78eaca146326606b > src/slave/flags.cpp cbf431eb0627bdaf07241cc0fc4630df06fb20e2 > > Diff: https://reviews.apache.org/r/34139/diff/ > > > Testing > ------- > > > Thanks, > > Ian Downes > >
