On June 10, 2015, 8:37 p.m., Ben Mahler wrote:
How about adding an 'Address' message, which can contain 'ip' and 'port'
for now?
```
message Address {
required string ip;
required uint32 port;
// Later we can add 'hostname' or 'public_hostname', etc!
}
```
On June 5, 2015, 6:42 p.m., Vinod Kone wrote:
I made some minor comments below but I think a better way to do this is to
*not* write custom masterinfo json - protobuf converters. I prefer we
just add a new optional field (say ipAddress of type string). Then you can
just leverage the
On June 5, 2015, 6:42 p.m., Vinod Kone wrote:
I made some minor comments below but I think a better way to do this is to
*not* write custom masterinfo json - protobuf converters. I prefer we
just add a new optional field (say ipAddress of type string). Then you can
just leverage the
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34687/#review87520
---
How about adding an 'Address' message, which can contain 'ip' and
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34687/#review86808
---
src/common/http.hpp
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34687/#review86819
---
I made some minor comments below but I think a better way to do
On June 5, 2015, 11:42 a.m., Vinod Kone wrote:
src/common/http.cpp, line 212
https://reviews.apache.org/r/34687/diff/6/?file=977637#file977637line212
Why do you need this temporary?
BenH brought this up - look above. I think this is fine if it is made const.
- Niklas
On June 5, 2015, 6:42 p.m., Vinod Kone wrote:
I made some minor comments below but I think a better way to do this is to
*not* write custom masterinfo json - protobuf converters. I prefer we
just add a new optional field (say ipAddress of type string). Then you can
just leverage the
On June 5, 2015, 6:42 p.m., Vinod Kone wrote:
I made some minor comments below but I think a better way to do this is to
*not* write custom masterinfo json - protobuf converters. I prefer we
just add a new optional field (say ipAddress of type string). Then you can
just leverage the
On June 5, 2015, 11:42 a.m., Vinod Kone wrote:
I made some minor comments below but I think a better way to do this is to
*not* write custom masterinfo json - protobuf converters. I prefer we
just add a new optional field (say ipAddress of type string). Then you can
just leverage the
On June 5, 2015, 6:42 p.m., Vinod Kone wrote:
I made some minor comments below but I think a better way to do this is to
*not* write custom masterinfo json - protobuf converters. I prefer we
just add a new optional field (say ipAddress of type string). Then you can
just leverage the
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34687/
---
(Updated June 5, 2015, 8:18 p.m.)
Review request for mesos, haosdent huang and
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34687/#review86453
---
Looking much better! :)
A few suggestions below and let's get this
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34687/#review86482
---
Patch looks great!
Reviews applied: [34687]
All tests passed.
-
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34687/#review86176
---
I haven't done a full review here but took interest in this because
On June 2, 2015, 9:10 a.m., Benjamin Hindman wrote:
src/common/http.cpp, lines 212-213
https://reviews.apache.org/r/34687/diff/4/?file=975084#file975084line212
Please see formatting for braces:
On June 2, 2015, 9:10 a.m., Benjamin Hindman wrote:
I haven't done a full review here but took interest in this because the
required 'ip' field of the MasterInfo protobuf (as well as other protobufs)
is deprecated because it doesn't support IPv6. We need to make 'ip' an
optional
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34687/#review86299
---
Patch looks great!
Reviews applied: [34687]
All tests passed.
-
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34687/#review85946
---
src/Makefile.am
https://reviews.apache.org/r/34687/#comment137765
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34687/#review85948
---
src/common/parse.hpp
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34687/#review85949
---
Ship it!
Ship It!
- haosdent huang
On May 31, 2015, 2:58 a.m.,
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34687/#review85980
---
src/common/parse.hpp
On June 1, 2015, 12:08 a.m., haosdent huang wrote:
src/Makefile.am, line 1465
https://reviews.apache.org/r/34687/diff/4/?file=975082#file975082line1465
seems the order here is follow dictionary order, so how about move them
after `tests/authorization_tests.cpp`?
Note that this
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34687/#review85893
---
Patch looks great!
Reviews applied: [34687]
All tests passed.
-
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34687/
---
(Updated May 31, 2015, 2:58 a.m.)
Review request for mesos, haosdent huang and
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34687/
---
(Updated May 29, 2015, 9:36 p.m.)
Review request for mesos, haosdent huang and
On May 27, 2015, 2:18 a.m., Niklas Nielsen wrote:
src/common/protobuf_utils.hpp, line 83
https://reviews.apache.org/r/34687/diff/1/?file=972329#file972329line83
Shouldn't parse() go in
https://github.com/apache/mesos/blob/master/src/common/parse.hpp?
Marco Massenzio wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34687/#review85340
---
Patch looks great!
Reviews applied: [34687]
All tests passed.
-
On May 27, 2015, 2:18 a.m., Niklas Nielsen wrote:
src/common/protobuf_utils.hpp, line 83
https://reviews.apache.org/r/34687/diff/1/?file=972329#file972329line83
Shouldn't parse() go in
https://github.com/apache/mesos/blob/master/src/common/parse.hpp?
I don't really know - in the
On May 27, 2015, 2:18 a.m., Niklas Nielsen wrote:
src/common/protobuf_utils.cpp, line 169
https://reviews.apache.org/r/34687/diff/1/?file=972330#file972330line169
We have traditionally used stringify(...) if we needed type T to string
conversions. Have you checked if we have one
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34687/#review85499
---
Patch looks great!
Reviews applied: [34687]
All tests passed.
-
On May 27, 2015, 2:18 a.m., Niklas Nielsen wrote:
src/Makefile.am, line 1387
https://reviews.apache.org/r/34687/diff/1/?file=972328#file972328line1387
Is the alignment right here? Try to set your tabstop to 8 :)
Also, shouldn't the ordering bring this further down?
no, it's
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34687/
---
Review request for mesos, haosdent huang and Niklas Nielsen.
Bugs: MESOS-2340
33 matches
Mail list logo
