----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49207/#review139774 -----------------------------------------------------------
include/mesos/appc/spec.proto (lines 44 - 46) <https://reviews.apache.org/r/49207/#comment205115> I saw that you dropped previous comments, but it would be great if you can show some comments to explain when you want to drop some comments ;-) I would suggest that you add a link as following here as comment so that people will clear where does those field from. https://github.com/appc/spec/blob/master/spec/aci.md#image-manifest-schema Other comments are: 1) Please always add a period for all of your comments. 2) Always start with a upper case charactor for a sentense, for your case, it would be /*TODO(srbrahma): Would xxxxx*/ , but I think the comments here needs to be updated. include/mesos/appc/spec.proto (line 50) <https://reviews.apache.org/r/49207/#comment205120> You may see that we have two similar message `Label` and `Annotation` message Label { required string name = 1; required string value = 2; } message Annotation { required string name = 1; required string value = 2; } For `environment` here, what about adding a new protobuf named as `Environment`? message Environment { required string name = 1; required string value = 2; } message App { repeated string exec = 1; optional string workingDirectory = 2; repeated Environment environment = 3; } include/mesos/slave/isolator.proto (line 92) <https://reviews.apache.org/r/49207/#comment205114> Per gilbert's comments, can you update this to `Appc` as https://github.com/apache/mesos/blob/master/include/mesos/v1/mesos.proto#L1605 - Guangya Liu On 六月 28, 2016, 5:05 a.m., Srinivas Brahmaroutu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/49207/ > ----------------------------------------------------------- > > (Updated 六月 28, 2016, 5:05 a.m.) > > > Review request for mesos, Gilbert Song and Jie Yu. > > > Bugs: MESOS-4778 > https://issues.apache.org/jira/browse/MESOS-4778 > > > Repository: mesos > > > Description > ------- > > Added proto message definitions to support appc runtime. > > > Diffs > ----- > > include/mesos/appc/spec.proto 4ef0473748c2f47aff1729c1d969185803fe72d4 > include/mesos/slave/isolator.proto f17a3a4f167eb203709d7ebac3ade220ac8641ea > > Diff: https://reviews.apache.org/r/49207/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Srinivas Brahmaroutu > >
