Re: Review Request 41380: Added repeated vip field to DiscoveryInfo and an instance_port field to Port

2015-12-19 Thread Anand Mazumdar
> On Dec. 19, 2015, 10:01 a.m., Adam B wrote: > > include/mesos/mesos.proto, lines 1576-1577 > > > > > > Do we expect to add more fields to the Vip message later? > > If not, why don't we just add a `repeated

Re: Review Request 41380: Added repeated vip field to DiscoveryInfo and an instance_port field to Port

2015-12-19 Thread Avinash sridharan
> On Dec. 19, 2015, 10:01 a.m., Adam B wrote: > > include/mesos/mesos.proto, lines 1576-1577 > > > > > > Do we expect to add more fields to the Vip message later? > > If not, why don't we just add a `repeated

Re: Review Request 41380: Added repeated vip field to DiscoveryInfo and an instance_port field to Port

2015-12-19 Thread Avinash sridharan
> On Dec. 19, 2015, 10:01 a.m., Adam B wrote: > > include/mesos/mesos.proto, lines 1576-1577 > > > > > > Do we expect to add more fields to the Vip message later? > > If not, why don't we just add a `repeated

Re: Review Request 40944: Fixed protobuf parse failure when pulling a docker image.

2015-12-19 Thread Gilbert Song
> On Dec. 18, 2015, 5:22 p.m., Timothy Chen wrote: > > src/slave/containerizer/mesos/provisioner/docker/message.proto, line 72 > > > > > > I think we should make this optional as well, since technically we > > don't

Re: Review Request 41380: Added repeated vip field to DiscoveryInfo and an instance_port field to Port

2015-12-19 Thread Avinash sridharan
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41380/ --- (Updated Dec. 20, 2015, 1:51 a.m.) Review request for mesos, Adam B and Anand

Re: Review Request 40838: Environment variable: Implemented `Env` specified in docker image returned from docker pull.

2015-12-19 Thread Gilbert Song
> On Dec. 18, 2015, 5:18 p.m., Timothy Chen wrote: > > src/docker/docker.cpp, line 407 > > > > > > Do we have a test for this? Yes, we do. I wrote a test case for it, but did not post that test. Becuase I am not

Re: Review Request 40944: Fixed protobuf parse failure when pulling a docker image.

2015-12-19 Thread Gilbert Song
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40944/ --- (Updated Dec. 19, 2015, 5:43 p.m.) Review request for mesos, Artem

Re: Review Request 41381: Added unit test cases to test the new vip and instance_port fields

2015-12-19 Thread Avinash sridharan
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41381/ --- (Updated Dec. 20, 2015, 1:56 a.m.) Review request for mesos, Adam B and Anand

Re: Review Request 40944: Fixed protobuf parse failure when pulling a docker image.

2015-12-19 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40944/#review111398 --- Patch looks great! Reviews applied: [40944] Passed command:

Re: Review Request 40838: Environment variable: Implemented `Env` specified in docker image returned from docker pull.

2015-12-19 Thread Gilbert Song
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40838/ --- (Updated Dec. 19, 2015, 5:35 p.m.) Review request for mesos, Artem Harutyunyan

Re: Review Request 41381: Added unit test cases to test the new vip and instance_port fields

2015-12-19 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41381/#review111399 --- Bad review! Reviews applied: [] Error: No reviewers specified.

Re: Review Request 41380: Added repeated vip field to DiscoveryInfo and an instance_port field to Port

2015-12-19 Thread Adam B
> On Dec. 18, 2015, 2:54 p.m., Artem Harutyunyan wrote: > > include/mesos/mesos.proto, line 1597 > > > > > > Why is it a Vip and not VIP, or VIp, or maybe even virtual_ip to be > > consistent with the style?

Re: Review Request 41381: Added unit test cases to test the new vip and instance_port fields

2015-12-19 Thread Adam B
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41381/#review111370 --- src/tests/slave_tests.cpp (line 2238)

Re: Review Request 41381: Added unit test cases to test the new vip and instance_port fields

2015-12-19 Thread Avinash sridharan
> On Dec. 19, 2015, 5:49 a.m., Artem Harutyunyan wrote: > > src/tests/slave_tests.cpp, lines 2242-2246 > > > > > > How about using EXPECT_SOME_EQ here? This is much cleaner. > On Dec. 19, 2015, 5:49 a.m., Artem

Re: Review Request 41380: Added repeated vip field to DiscoveryInfo and an instance_port field to Port

2015-12-19 Thread Adam B
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41380/#review111369 --- Please rename `Vip` back, or go with `VIP`. Or get rid of that

Re: Review Request 41381: Added unit test cases to test the new vip and instance_port fields

2015-12-19 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41381/#review111367 --- Patch looks great! Reviews applied: [41380, 41381] Passed

Review Request 41584: Added commit message guidelines to docs.

2015-12-19 Thread Artem Harutyunyan
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41584/ --- Review request for mesos, Adam B, Benjamin Hindman, and Neil Conway.

Review Request 41586: Partially enforced commit message guidelines with a hook.

2015-12-19 Thread Artem Harutyunyan
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41586/ --- Review request for mesos, Adam B, Benjamin Hindman, and Neil Conway.

Re: Review Request 41586: Partially enforced commit message guidelines with a hook.

2015-12-19 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41586/#review111371 --- Patch looks great! Reviews applied: [41584, 41586] Passed

Re: Review Request 41381: Added unit test cases to test the new vip and instance_port fields

2015-12-19 Thread Avinash sridharan
> On Dec. 19, 2015, 10:10 a.m., Adam B wrote: > > src/tests/slave_tests.cpp, lines 2248-2253 > > > > > > EXPECT_SOME_EQ here too. Since this was your own test that we just > > committed, let's fix it ourselves

Re: Review Request 41584: Added commit message guidelines to docs.

2015-12-19 Thread Till Toenshoff
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41584/#review111372 --- docs/submitting-a-patch.md (line 52)