----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37773/#review96905 -----------------------------------------------------------
src/slave/containerizer/provisioners/docker/registry_client.hpp (line 46) <https://reviews.apache.org/r/37773/#comment152585> Why not just use option.getOrElse()? src/slave/containerizer/provisioners/docker/registry_client.hpp (line 77) <https://reviews.apache.org/r/37773/#comment152586> userId src/slave/containerizer/provisioners/docker/registry_client.hpp (line 116) <https://reviews.apache.org/r/37773/#comment152587> We usually spell timeout as one word everywhere else, so all lower case. src/slave/containerizer/provisioners/docker/registry_client.hpp (line 150) <https://reviews.apache.org/r/37773/#comment152588> Do we need to define static here if it's not being used anywhere else but the cpp files? src/slave/containerizer/provisioners/docker/registry_client.cpp (line 41) <https://reviews.apache.org/r/37773/#comment152589> Kill extra line src/slave/containerizer/provisioners/docker/registry_client.cpp (line 87) <https://reviews.apache.org/r/37773/#comment152593> What's the reasoning behind prefixing underscores for these? You've already added trailing underscores for your class variables right? src/slave/containerizer/provisioners/docker/registry_client.cpp (line 91) <https://reviews.apache.org/r/37773/#comment152590> getOrElse src/slave/containerizer/provisioners/docker/registry_client.cpp (line 114) <https://reviews.apache.org/r/37773/#comment152592> I'm wondering why you need to use promise here. If you put all this logic in getManifest then essentially you just need to return the dispatch right? And in getManifest you just chian your return with onFailed and onAny too. Also even if you don't move it in getManifest why can't you just return result.onFailed(...).onAny(...)? src/slave/containerizer/provisioners/docker/registry_client.cpp (line 153) <https://reviews.apache.org/r/37773/#comment152594> What residue? - Timothy Chen On Aug. 28, 2015, 6:38 p.m., Jojy Varghese wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/37773/ > ----------------------------------------------------------- > > (Updated Aug. 28, 2015, 6:38 p.m.) > > > Review request for mesos, Lily Chen and Timothy Chen. > > > Repository: mesos > > > Description > ------- > > Added implementation for docker registry's Get Manifest and Get Blob APIs. > > > Diffs > ----- > > src/Makefile.am 7b620ff66856b3f0adac121b3297d55ed71a3d99 > src/slave/containerizer/provisioners/docker/registry_client.hpp > PRE-CREATION > src/slave/containerizer/provisioners/docker/registry_client.cpp > PRE-CREATION > src/tests/provisioners/docker_provisioner_tests.cpp PRE-CREATION > > Diff: https://reviews.apache.org/r/37773/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Jojy Varghese > >
