----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52349/#review160842 -----------------------------------------------------------
Fix it, then Ship it! include/mesos/oci/spec.proto (lines 33 - 45) <https://reviews.apache.org/r/52349/#comment232079> See my comments below. No need for these two. include/mesos/oci/spec.proto (line 68) <https://reviews.apache.org/r/52349/#comment232073> I would move this out to top level. Similar to what they did in go. https://github.com/opencontainers/image-spec/blob/master/specs-go/v1/manifest_list.go This way, it's more consistent with upstream. include/mesos/oci/spec.proto (line 72) <https://reviews.apache.org/r/52349/#comment232075> Add a NOTE here. include/mesos/oci/spec.proto (line 81) <https://reviews.apache.org/r/52349/#comment232074> I'd pull this up to the top level and rename it to ManifestDescriptor. include/mesos/oci/spec.proto (line 144) <https://reviews.apache.org/r/52349/#comment232077> Given that we need to parse manually anyway, we can just use a repeated string here: ``` repeated string exposedPorts; ``` include/mesos/oci/spec.proto (line 152) <https://reviews.apache.org/r/52349/#comment232078> Ditto. Can this just be a repeated string? - Jie Yu On Jan. 3, 2017, 1:54 p.m., Qian Zhang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/52349/ > ----------------------------------------------------------- > > (Updated Jan. 3, 2017, 1:54 p.m.) > > > Review request for mesos, Gilbert Song and Jie Yu. > > > Bugs: MESOS-6681 > https://issues.apache.org/jira/browse/MESOS-6681 > > > Repository: mesos > > > Description > ------- > > Add protobuf messages for OCI image spec. > > > Diffs > ----- > > include/mesos/oci/spec.hpp PRE-CREATION > include/mesos/oci/spec.proto PRE-CREATION > src/Makefile.am 6d0f77be37af9bc4e22199418796d6d0c5b6c462 > > Diff: https://reviews.apache.org/r/52349/diff/ > > > Testing > ------- > > > Thanks, > > Qian Zhang > >