Re: Review Request 38901: Serialize Docker Image Spec as Protobuf

2015-10-15 Thread Timothy Chen 


> On Oct. 9, 2015, 8:12 p.m., Anand Mazumdar wrote:
> > src/slave/containerizer/provisioner/docker/spec.hpp, line 22
> > 
> >
> > Do we need this ? If not, remove this include.
> 
> Gilbert Song wrote:
> Ditto. It is used in the next patch.

You should only include it in the next patch then. I'll be merging both patches 
together this time but please only include what you need per patch.


- Timothy


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38901/#review102105
---


On Oct. 9, 2015, 9:39 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38901/
> ---
> 
> (Updated Oct. 9, 2015, 9:39 p.m.)
> 
> 
> Review request for mesos, Jojy Varghese and Timothy Chen.
> 
> 
> Bugs: MESOS-2972
> https://issues.apache.org/jira/browse/MESOS-2972
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Serialize Docker Image Spec as Protobuf
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 828dbb690841c561816811dfbb044aa3afead89d 
>   src/Makefile.am d855cb83277c3e0e2ee3feacaf6ad0962223ef6e 
>   src/slave/containerizer/provisioner/docker/message.proto 
> bbac2e6c1f40a7ca3f9227baca56a44cd43f58c6 
>   src/slave/containerizer/provisioner/docker/spec.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/spec.cpp PRE-CREATION 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> d895eb9d0723e52cff8b21ef2deeaef1911d019c 
> 
> Diff: https://reviews.apache.org/r/38901/diff/
> 
> 
> Testing
> ---
> 
> make check (ubuntu 14.04 + clang++-3.6)
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 38901: Serialize Docker Image Spec as Protobuf

2015-10-12 Thread Gilbert Song 


> On Oct. 9, 2015, 1:12 p.m., Anand Mazumdar wrote:
> > src/slave/containerizer/provisioner/docker/spec.hpp, line 18
> > 
> >
> > We generally prefer header files that are `complete` i.e. they compile 
> > on their own. Can you ensure that we include `stout/json.hpp` here since it 
> > has `JSON::Object`. Also , do a sweep of the other includes.

Thank you for reminding. For your recommendation of sweeping, the others are 
included and used in the next patch, which is depending this one. So I leave 
them here.


> On Oct. 9, 2015, 1:12 p.m., Anand Mazumdar wrote:
> > src/slave/containerizer/provisioner/docker/spec.hpp, line 22
> > 
> >
> > Do we need this ? If not, remove this include.

Ditto. It is used in the next patch.


> On Oct. 9, 2015, 1:12 p.m., Anand Mazumdar wrote:
> > src/slave/containerizer/provisioner/docker/spec.cpp, line 20
> > 
> >
> > Do you need this ? If not remove it

Ditto. (referring to https://reviews.apache.org/r/38919/diff/4#index_header)


> On Oct. 9, 2015, 1:12 p.m., Anand Mazumdar wrote:
> > src/slave/containerizer/provisioner/docker/spec.cpp, line 24
> > 
> >
> > Do you need this ? If not remove it

ditto.


> On Oct. 9, 2015, 1:12 p.m., Anand Mazumdar wrote:
> > src/tests/containerizer/provisioner_docker_tests.cpp, line 331
> > 
> >
> > Nit: Does it make sense to change the ASSERT_EQ to EXPECT_EQ ? 
> > 
> > Might help the user to see a list of all the things that failed instead 
> > of just aborting at the first failure.

This is a good point.


- Gilbert


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38901/#review102105
---


On Oct. 9, 2015, 2:39 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38901/
> ---
> 
> (Updated Oct. 9, 2015, 2:39 p.m.)
> 
> 
> Review request for mesos, Jojy Varghese and Timothy Chen.
> 
> 
> Bugs: MESOS-2972
> https://issues.apache.org/jira/browse/MESOS-2972
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Serialize Docker Image Spec as Protobuf
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 828dbb690841c561816811dfbb044aa3afead89d 
>   src/Makefile.am d855cb83277c3e0e2ee3feacaf6ad0962223ef6e 
>   src/slave/containerizer/provisioner/docker/message.proto 
> bbac2e6c1f40a7ca3f9227baca56a44cd43f58c6 
>   src/slave/containerizer/provisioner/docker/spec.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/spec.cpp PRE-CREATION 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> d895eb9d0723e52cff8b21ef2deeaef1911d019c 
> 
> Diff: https://reviews.apache.org/r/38901/diff/
> 
> 
> Testing
> ---
> 
> make check (ubuntu 14.04 + clang++-3.6)
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 38901: Serialize Docker Image Spec as Protobuf

2015-10-09 Thread Gilbert Song 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38901/
---

(Updated Oct. 9, 2015, 2:39 p.m.)


Review request for mesos, Jojy Varghese and Timothy Chen.


Bugs: MESOS-2972
https://issues.apache.org/jira/browse/MESOS-2972


Repository: mesos


Description
---

Serialize Docker Image Spec as Protobuf


Diffs (updated)
-

  src/CMakeLists.txt 828dbb690841c561816811dfbb044aa3afead89d 
  src/Makefile.am d855cb83277c3e0e2ee3feacaf6ad0962223ef6e 
  src/slave/containerizer/provisioner/docker/message.proto 
bbac2e6c1f40a7ca3f9227baca56a44cd43f58c6 
  src/slave/containerizer/provisioner/docker/spec.hpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/spec.cpp PRE-CREATION 
  src/tests/containerizer/provisioner_docker_tests.cpp 
d895eb9d0723e52cff8b21ef2deeaef1911d019c 

Diff: https://reviews.apache.org/r/38901/diff/


Testing
---

make check (ubuntu 14.04 + clang++-3.6)


Thanks,

Gilbert Song

Re: Review Request 38901: Serialize Docker Image Spec as Protobuf

2015-10-09 Thread Timothy Chen 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38901/#review102103
---

Ship it!


Ship It!


src/tests/containerizer/provisioner_docker_tests.cpp (line 241)


You don't need to use TemporaryDirectoryTest since you're not writing any 
files.

You probably can just use ::testing::Test as base


- Timothy Chen


On Oct. 7, 2015, 6:57 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38901/
> ---
> 
> (Updated Oct. 7, 2015, 6:57 p.m.)
> 
> 
> Review request for mesos, Jojy Varghese and Timothy Chen.
> 
> 
> Bugs: MESOS-2972
> https://issues.apache.org/jira/browse/MESOS-2972
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Serialize Docker Image Spec as Protobuf
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 828dbb690841c561816811dfbb044aa3afead89d 
>   src/Makefile.am e69892736b0edc8c264eaccd52a04d44d01f53ba 
>   src/slave/containerizer/provisioner/docker/message.proto 
> bbac2e6c1f40a7ca3f9227baca56a44cd43f58c6 
>   src/slave/containerizer/provisioner/docker/spec.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/spec.cpp PRE-CREATION 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> d895eb9d0723e52cff8b21ef2deeaef1911d019c 
> 
> Diff: https://reviews.apache.org/r/38901/diff/
> 
> 
> Testing
> ---
> 
> make check (ubuntu 14.04 + clang++-3.6)
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 38901: Serialize Docker Image Spec as Protobuf

2015-10-09 Thread Anand Mazumdar 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38901/#review102105
---


LGTM, Just some minor comments/concerns about including `headers` that we 
need/style issues. Can you do a sweep of the `includes` in case I missed 
pointing some of them here.


src/Makefile.am (line 852)


Nit: Can we align the trailing `` ?



src/slave/containerizer/provisioner/docker/spec.hpp (line 18)


We generally prefer header files that are `complete` i.e. they compile on 
their own. Can you ensure that we include `stout/json.hpp` here since it has 
`JSON::Object`. Also , do a sweep of the other includes.



src/slave/containerizer/provisioner/docker/spec.hpp (line 22)


Do we need this ? If not, remove this include.



src/slave/containerizer/provisioner/docker/spec.cpp (line 20)


Do you need this ? If not remove it



src/slave/containerizer/provisioner/docker/spec.cpp (line 24)


Do you need this ? If not remove it



src/tests/containerizer/provisioner_docker_tests.cpp (line 240)


Remove this comment. It's Redundant/self-explanatory here.



src/tests/containerizer/provisioner_docker_tests.cpp (line 325)


s/EXPECT_SOME/ASSERT_SOME , since it makes no sense to continue this test 
if this fails.



src/tests/containerizer/provisioner_docker_tests.cpp (line 331)


Nit: Does it make sense to change the ASSERT_EQ to EXPECT_EQ ? 

Might help the user to see a list of all the things that failed instead of 
just aborting at the first failure.



src/tests/containerizer/provisioner_docker_tests.cpp (line 357)


How about :

`// This is an invalid manifest. The required fields 'signatures' and 
'schemaVersion' are not set.`

While testing, we only make assumptions about the things we are testing as 
part of our test. The `validateManifest` call is not part of this test, so its 
better to just ignore/not mention it.


- Anand Mazumdar


On Oct. 7, 2015, 6:57 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38901/
> ---
> 
> (Updated Oct. 7, 2015, 6:57 p.m.)
> 
> 
> Review request for mesos, Jojy Varghese and Timothy Chen.
> 
> 
> Bugs: MESOS-2972
> https://issues.apache.org/jira/browse/MESOS-2972
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Serialize Docker Image Spec as Protobuf
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 828dbb690841c561816811dfbb044aa3afead89d 
>   src/Makefile.am e69892736b0edc8c264eaccd52a04d44d01f53ba 
>   src/slave/containerizer/provisioner/docker/message.proto 
> bbac2e6c1f40a7ca3f9227baca56a44cd43f58c6 
>   src/slave/containerizer/provisioner/docker/spec.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/spec.cpp PRE-CREATION 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> d895eb9d0723e52cff8b21ef2deeaef1911d019c 
> 
> Diff: https://reviews.apache.org/r/38901/diff/
> 
> 
> Testing
> ---
> 
> make check (ubuntu 14.04 + clang++-3.6)
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 38901: Serialize Docker Image Spec as Protobuf

2015-10-06 Thread Gilbert Song 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38901/
---

(Updated Oct. 6, 2015, 10:29 a.m.)


Review request for mesos, Jojy Varghese and Timothy Chen.


Bugs: MESOS-2972
https://issues.apache.org/jira/browse/MESOS-2972


Repository: mesos


Description
---

Serialize Docker Image Spec as Protobuf


Diffs (updated)
-

  src/CMakeLists.txt 828dbb690841c561816811dfbb044aa3afead89d 
  src/Makefile.am e69892736b0edc8c264eaccd52a04d44d01f53ba 
  src/slave/containerizer/provisioner/docker/message.proto 
bbac2e6c1f40a7ca3f9227baca56a44cd43f58c6 
  src/slave/containerizer/provisioner/docker/spec.hpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/spec.cpp PRE-CREATION 
  src/tests/containerizer/provisioner_docker_tests.cpp 
d895eb9d0723e52cff8b21ef2deeaef1911d019c 

Diff: https://reviews.apache.org/r/38901/diff/


Testing
---

make check (ubuntu 14.04 + clang++-3.6)


Thanks,

Gilbert Song

Re: Review Request 38901: Serialize Docker Image Spec as Protobuf

2015-10-05 Thread Gilbert Song 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38901/
---

(Updated Oct. 5, 2015, 12:21 p.m.)


Review request for mesos, Jojy Varghese and Timothy Chen.


Changes
---

delete unnecessary protobuf


Bugs: MESOS-2972
https://issues.apache.org/jira/browse/MESOS-2972


Repository: mesos


Description
---

Serialize Docker Image Spec as Protobuf


Diffs (updated)
-

  src/CMakeLists.txt 891f951f3107ece29b7923b7a3cc414e2ea56983 
  src/Makefile.am f060998bb08cdb071db5a2e85dfbad805dab45e9 
  src/slave/containerizer/provisioner/docker/message.proto 
bbac2e6c1f40a7ca3f9227baca56a44cd43f58c6 
  src/slave/containerizer/provisioner/docker/spec.hpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/spec.cpp PRE-CREATION 
  src/tests/containerizer/provisioner_docker_tests.cpp 
d895eb9d0723e52cff8b21ef2deeaef1911d019c 

Diff: https://reviews.apache.org/r/38901/diff/


Testing
---

make check (ubuntu 14.04 + clang++-3.6)


Thanks,

Gilbert Song

Re: Review Request 38901: Serialize Docker Image Spec as Protobuf

2015-10-05 Thread Gilbert Song 


> On Oct. 2, 2015, 3:38 p.m., Timothy Chen wrote:
> > src/tests/containerizer/provisioner_docker_tests.cpp, line 361
> > 
> >
> > What's invalid about this JSON?

Because we have to make sure that we have at least one fsLayer blobSum and 
corresponding v1Compatibility, which is validated from validateManifest(const 
DockerImageManifest& manifest).
For here, history, fsLayers and signatures are missed.


- Gilbert


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38901/#review101387
---


On Oct. 5, 2015, 12:14 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38901/
> ---
> 
> (Updated Oct. 5, 2015, 12:14 p.m.)
> 
> 
> Review request for mesos, Jojy Varghese and Timothy Chen.
> 
> 
> Bugs: MESOS-2972
> https://issues.apache.org/jira/browse/MESOS-2972
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Serialize Docker Image Spec as Protobuf
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 891f951f3107ece29b7923b7a3cc414e2ea56983 
>   src/Makefile.am f060998bb08cdb071db5a2e85dfbad805dab45e9 
>   src/slave/containerizer/provisioner/docker/message.proto 
> bbac2e6c1f40a7ca3f9227baca56a44cd43f58c6 
>   src/slave/containerizer/provisioner/docker/spec.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/spec.cpp PRE-CREATION 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> d895eb9d0723e52cff8b21ef2deeaef1911d019c 
> 
> Diff: https://reviews.apache.org/r/38901/diff/
> 
> 
> Testing
> ---
> 
> make check (ubuntu 14.04 + clang++-3.6)
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 38901: Serialize Docker Image Spec as Protobuf

2015-10-05 Thread Gilbert Song 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38901/
---

(Updated Oct. 5, 2015, 12:14 p.m.)


Review request for mesos, Jojy Varghese and Timothy Chen.


Bugs: MESOS-2972
https://issues.apache.org/jira/browse/MESOS-2972


Repository: mesos


Description
---

Serialize Docker Image Spec as Protobuf


Diffs (updated)
-

  src/CMakeLists.txt 891f951f3107ece29b7923b7a3cc414e2ea56983 
  src/Makefile.am f060998bb08cdb071db5a2e85dfbad805dab45e9 
  src/slave/containerizer/provisioner/docker/message.proto 
bbac2e6c1f40a7ca3f9227baca56a44cd43f58c6 
  src/slave/containerizer/provisioner/docker/spec.hpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/spec.cpp PRE-CREATION 
  src/tests/containerizer/provisioner_docker_tests.cpp 
d895eb9d0723e52cff8b21ef2deeaef1911d019c 

Diff: https://reviews.apache.org/r/38901/diff/


Testing
---

make check (ubuntu 14.04 + clang++-3.6)


Thanks,

Gilbert Song

Re: Review Request 38901: Serialize Docker Image Spec as Protobuf

2015-10-05 Thread Timothy Chen 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38901/#review101533
---



src/slave/containerizer/provisioner/docker/message.proto (line 79)


End all comments with period. And add a space in the beginning:

// JSON Web key.


- Timothy Chen


On Oct. 5, 2015, 7:21 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38901/
> ---
> 
> (Updated Oct. 5, 2015, 7:21 p.m.)
> 
> 
> Review request for mesos, Jojy Varghese and Timothy Chen.
> 
> 
> Bugs: MESOS-2972
> https://issues.apache.org/jira/browse/MESOS-2972
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Serialize Docker Image Spec as Protobuf
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 891f951f3107ece29b7923b7a3cc414e2ea56983 
>   src/Makefile.am f060998bb08cdb071db5a2e85dfbad805dab45e9 
>   src/slave/containerizer/provisioner/docker/message.proto 
> bbac2e6c1f40a7ca3f9227baca56a44cd43f58c6 
>   src/slave/containerizer/provisioner/docker/spec.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/spec.cpp PRE-CREATION 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> d895eb9d0723e52cff8b21ef2deeaef1911d019c 
> 
> Diff: https://reviews.apache.org/r/38901/diff/
> 
> 
> Testing
> ---
> 
> make check (ubuntu 14.04 + clang++-3.6)
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 38901: Serialize Docker Image Spec as Protobuf

2015-10-05 Thread Timothy Chen 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38901/#review101553
---



src/tests/containerizer/provisioner_docker_tests.cpp (line 357)


Add a comment in the code to say why this is invalid.


- Timothy Chen


On Oct. 5, 2015, 7:21 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38901/
> ---
> 
> (Updated Oct. 5, 2015, 7:21 p.m.)
> 
> 
> Review request for mesos, Jojy Varghese and Timothy Chen.
> 
> 
> Bugs: MESOS-2972
> https://issues.apache.org/jira/browse/MESOS-2972
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Serialize Docker Image Spec as Protobuf
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 891f951f3107ece29b7923b7a3cc414e2ea56983 
>   src/Makefile.am f060998bb08cdb071db5a2e85dfbad805dab45e9 
>   src/slave/containerizer/provisioner/docker/message.proto 
> bbac2e6c1f40a7ca3f9227baca56a44cd43f58c6 
>   src/slave/containerizer/provisioner/docker/spec.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/spec.cpp PRE-CREATION 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> d895eb9d0723e52cff8b21ef2deeaef1911d019c 
> 
> Diff: https://reviews.apache.org/r/38901/diff/
> 
> 
> Testing
> ---
> 
> make check (ubuntu 14.04 + clang++-3.6)
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 38901: Serialize Docker Image Spec as Protobuf

2015-10-02 Thread Gilbert Song 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38901/
---

(Updated Oct. 2, 2015, 1:10 p.m.)


Review request for mesos, Jojy Varghese and Timothy Chen.


Bugs: MESOS-2972
https://issues.apache.org/jira/browse/MESOS-2972


Repository: mesos


Description
---

Serialize Docker Image Spec as Protobuf


Diffs (updated)
-

  src/CMakeLists.txt 891f951f3107ece29b7923b7a3cc414e2ea56983 
  src/Makefile.am f060998bb08cdb071db5a2e85dfbad805dab45e9 
  src/slave/containerizer/provisioner/docker/message.proto 
bbac2e6c1f40a7ca3f9227baca56a44cd43f58c6 
  src/slave/containerizer/provisioner/docker/spec.hpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/spec.cpp PRE-CREATION 
  src/tests/containerizer/provisioner_docker_tests.cpp 
d895eb9d0723e52cff8b21ef2deeaef1911d019c 

Diff: https://reviews.apache.org/r/38901/diff/


Testing
---

make check (ubuntu 14.04 + clang++-3.6)


Thanks,

Gilbert Song

Re: Review Request 38901: Serialize Docker Image Spec as Protobuf

2015-10-02 Thread Timothy Chen 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38901/#review101387
---



src/slave/containerizer/provisioner/docker/spec.hpp (line 37)


End all comments with a period.



src/tests/containerizer/provisioner_docker_tests.cpp (line 321)


No need to abort, just use ASSERT_SOME



src/tests/containerizer/provisioner_docker_tests.cpp (line 357)


How about call it invalid instead of insufficient



src/tests/containerizer/provisioner_docker_tests.cpp (line 361)


What's invalid about this JSON?



src/tests/containerizer/provisioner_docker_tests.cpp (line 368)


ditto


- Timothy Chen


On Oct. 2, 2015, 8:10 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38901/
> ---
> 
> (Updated Oct. 2, 2015, 8:10 p.m.)
> 
> 
> Review request for mesos, Jojy Varghese and Timothy Chen.
> 
> 
> Bugs: MESOS-2972
> https://issues.apache.org/jira/browse/MESOS-2972
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Serialize Docker Image Spec as Protobuf
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 891f951f3107ece29b7923b7a3cc414e2ea56983 
>   src/Makefile.am f060998bb08cdb071db5a2e85dfbad805dab45e9 
>   src/slave/containerizer/provisioner/docker/message.proto 
> bbac2e6c1f40a7ca3f9227baca56a44cd43f58c6 
>   src/slave/containerizer/provisioner/docker/spec.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/spec.cpp PRE-CREATION 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> d895eb9d0723e52cff8b21ef2deeaef1911d019c 
> 
> Diff: https://reviews.apache.org/r/38901/diff/
> 
> 
> Testing
> ---
> 
> make check (ubuntu 14.04 + clang++-3.6)
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 38901: Serialize Docker Image Spec as Protobuf

2015-09-30 Thread Alex Clemmer 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38901/#review101139
---



src/Makefile.am (line 537)


Could we please add this file to the CMakeLists file in src/ also?


- Alex Clemmer


On Sept. 30, 2015, 6:31 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38901/
> ---
> 
> (Updated Sept. 30, 2015, 6:31 p.m.)
> 
> 
> Review request for mesos, Jojy Varghese and Timothy Chen.
> 
> 
> Bugs: MESOS-2972
> https://issues.apache.org/jira/browse/MESOS-2972
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Serialize Docker Image Spec as Protobuf
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 8aa456611dd5405336dd7b0c19ba4a942ea1c805 
>   src/slave/containerizer/provisioner/docker/message.proto 
> bbac2e6c1f40a7ca3f9227baca56a44cd43f58c6 
>   src/slave/containerizer/provisioner/docker/spec.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/spec.cpp PRE-CREATION 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> d895eb9d0723e52cff8b21ef2deeaef1911d019c 
> 
> Diff: https://reviews.apache.org/r/38901/diff/
> 
> 
> Testing
> ---
> 
> make check (ubuntu 14.04 + clang++-3.6)
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 38901: Serialize Docker Image Spec as Protobuf

2015-09-30 Thread Mesos ReviewBot 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38901/#review101156
---


Patch looks great!

Reviews applied: [38901]

All tests passed.

- Mesos ReviewBot


On Sept. 30, 2015, 6:31 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38901/
> ---
> 
> (Updated Sept. 30, 2015, 6:31 p.m.)
> 
> 
> Review request for mesos, Jojy Varghese and Timothy Chen.
> 
> 
> Bugs: MESOS-2972
> https://issues.apache.org/jira/browse/MESOS-2972
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Serialize Docker Image Spec as Protobuf
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 8aa456611dd5405336dd7b0c19ba4a942ea1c805 
>   src/slave/containerizer/provisioner/docker/message.proto 
> bbac2e6c1f40a7ca3f9227baca56a44cd43f58c6 
>   src/slave/containerizer/provisioner/docker/spec.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner/docker/spec.cpp PRE-CREATION 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> d895eb9d0723e52cff8b21ef2deeaef1911d019c 
> 
> Diff: https://reviews.apache.org/r/38901/diff/
> 
> 
> Testing
> ---
> 
> make check (ubuntu 14.04 + clang++-3.6)
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>