----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34136/#review91666 -----------------------------------------------------------
Sorry I didn't notice it in my original review but I traced the issue back from future reviews. include/mesos/mesos.proto (lines 1211 - 1213) <https://reviews.apache.org/r/34136/#comment145213> So I found the use of the field `id` inconsistent in the code. Sometimes `id` has the `sha512-` prefix and sometimes not. I think we should consistently refer to `id` using the definition in the [spec](https://github.com/appc/spec/blob/806b17c86ba5e5d595fca3f7ed339c8a22fb46c3/spec/aci.md#image-id), i.e., with the prefix. The fact that the ID is computed by the image creator using sha512 and that the provisioner validates it using sha512 is merely an implementation detail that is not a conern of higher level abstractions / APIs. So here I think in the comments we should not call it "Image hash" but rather refer to the spec for its full definition. We can of course call out the fact that it should have the "sha512-" perfix. What do you think? - Jiang Yan Xu On July 11, 2015, 9:47 p.m., Ian Downes wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/34136/ > ----------------------------------------------------------- > > (Updated July 11, 2015, 9:47 p.m.) > > > Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone. > > > Repository: mesos > > > Description > ------- > > Add ContainerImage protobuf. > > > Diffs > ----- > > include/mesos/mesos.proto 1763129da535561503e89cbd8c4a371f8553d8d6 > > Diff: https://reviews.apache.org/r/34136/diff/ > > > Testing > ------- > > > Thanks, > > Ian Downes > >