Re: Review Request 59987: Added protobuf map support to stout JSON<->protobuf conversion.

2018-03-13 Thread Chun-Hung Hsiao
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59987/#review199103 --- Ship it! Ship It! - Chun-Hung Hsiao On March 13, 2018, 9:13

Re: Review Request 59987: Added protobuf map support to stout JSON<->protobuf conversion.

2018-03-13 Thread Qian Zhang
> On March 13, 2018, 10:07 a.m., Chun-Hung Hsiao wrote: > > 3rdparty/stout/include/stout/protobuf.hpp > > Lines 807-810 (patched) > > > > > > How about making this a lambda inside `protobuf()` so we don't expose >

Re: Review Request 59987: Added protobuf map support to stout JSON<->protobuf conversion.

2018-03-13 Thread Qian Zhang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59987/ --- (Updated March 13, 2018, 5:13 p.m.) Review request for mesos, Anand Mazumdar,

Re: Review Request 59987: Added protobuf map support to stout JSON<->protobuf conversion.

2018-03-12 Thread Chun-Hung Hsiao
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59987/#review199052 --- 3rdparty/stout/include/stout/protobuf.hpp Lines 807-810

Re: Review Request 59987: Added protobuf map support to stout JSON<->protobuf conversion.

2018-03-12 Thread Qian Zhang
> On March 1, 2018, 12:10 p.m., Benjamin Mahler wrote: > > 3rdparty/stout/include/stout/protobuf.hpp > > Lines 391-487 (patched) > > > > > > It looks like we can avoid this logic since protobuf's JSON conversion >

Re: Review Request 59987: Added protobuf map support to stout JSON<->protobuf conversion.

2018-03-12 Thread Qian Zhang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59987/ --- (Updated March 12, 2018, 3:15 p.m.) Review request for mesos, Anand Mazumdar,

Re: Review Request 59987: Added protobuf map support to stout JSON<->protobuf conversion.

2018-03-09 Thread Qian Zhang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59987/ --- (Updated March 9, 2018, 10:08 p.m.) Review request for mesos, Anand Mazumdar,

Re: Review Request 59987: Added protobuf map support to stout JSON<->protobuf conversion.

2018-03-09 Thread Qian Zhang
> On March 1, 2018, 12:10 p.m., Benjamin Mahler wrote: > > 3rdparty/stout/include/stout/protobuf.hpp > > Lines 391-487 (patched) > > > > > > It looks like we can avoid this logic since protobuf's JSON conversion >

Re: Review Request 59987: Added protobuf map support to stout JSON<->protobuf conversion.

2018-03-07 Thread Chun-Hung Hsiao
> On March 1, 2018, 4:10 a.m., Benjamin Mahler wrote: > > 3rdparty/stout/include/stout/protobuf.hpp > > Lines 388 (patched) > > > > > > s/name/key/? > > Qian Zhang wrote: > I'd like to be consistent with the

Re: Review Request 59987: Added protobuf map support to stout JSON<->protobuf conversion.

2018-03-06 Thread Qian Zhang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59987/ --- (Updated March 6, 2018, 5:29 p.m.) Review request for mesos, Anand Mazumdar,

Re: Review Request 59987: Added protobuf map support.

2018-03-06 Thread Qian Zhang
> On March 1, 2018, 12:10 p.m., Benjamin Mahler wrote: > > Chun and I went over this together, so feel free to reach out to either of > > us for discussion! Just some suggestions to clean up the code below. > > > > In the description, could we mention that this is for stout's json <-> > >

Re: Review Request 59987: Added protobuf map support.

2018-02-28 Thread Benjamin Mahler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59987/#review198429 --- Chun and I went over this together, so feel free to reach out to

Re: Review Request 59987: Added protobuf map support.

2018-02-26 Thread Chun-Hung Hsiao
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59987/#review198292 --- Ship it! Ship It! - Chun-Hung Hsiao On Feb. 26, 2018, 9:05

Re: Review Request 59987: Added protobuf map support.

2018-02-26 Thread Qian Zhang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59987/ --- (Updated Feb. 26, 2018, 5:05 p.m.) Review request for mesos, Anand Mazumdar,

Re: Review Request 59987: Added protobuf map support.

2018-02-26 Thread Qian Zhang
> On Feb. 9, 2018, 4:26 a.m., Chun-Hung Hsiao wrote: > > 3rdparty/stout/include/stout/protobuf.hpp > > Lines 499-500 (patched) > > > > > > How about using `UNREACHABLE` instead? With `ABORT`, we can output

Re: Review Request 59987: Added protobuf map support.

2018-02-08 Thread Chun-Hung Hsiao
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59987/#review197100 --- Overall good. Thanks for your efforts!

Re: Review Request 59987: Added protobuf map support.

2018-02-08 Thread Chun-Hung Hsiao
> On Sept. 22, 2017, 11:14 p.m., Chun-Hung Hsiao wrote: > > 3rdparty/stout/include/stout/protobuf.hpp > > Line 607 (original), 640 (patched) > > > > > > How about a partial specialization for

Re: Review Request 59987: Added protobuf map support.

2017-12-26 Thread Qian Zhang
> On Oct. 17, 2017, 7:15 p.m., Benjamin Bannier wrote: > > 3rdparty/stout/include/stout/protobuf.hpp > > Line 1 (original), 1 (patched) > > > > > > We should call out that we have a hard requirement on proto3 now in

Re: Review Request 59987: Added protobuf map support.

2017-12-20 Thread Qian Zhang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59987/ --- (Updated Dec. 21, 2017, 10:07 a.m.) Review request for mesos, Anand Mazumdar,

Re: Review Request 59987: Added protobuf map support.

2017-12-19 Thread Qian Zhang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59987/ --- (Updated Dec. 19, 2017, 9:50 p.m.) Review request for mesos, Anand Mazumdar

Re: Review Request 59987: Added protobuf map support.

2017-12-19 Thread Qian Zhang
> On Sept. 23, 2017, 7:14 a.m., Chun-Hung Hsiao wrote: > > 3rdparty/stout/include/stout/protobuf.hpp > > Line 607 (original), 640 (patched) > > > > > > How about a partial specialization for `google::protobuf::Map`?

Re: Review Request 59987: Added protobuf map support.

2017-10-17 Thread Qian Zhang
> On Oct. 17, 2017, 7:15 p.m., Benjamin Bannier wrote: > > 3rdparty/stout/include/stout/protobuf.hpp > > Line 1 (original), 1 (patched) > > > > > > We should call out that we have a hard requirement on proto3 now in

Re: Review Request 59987: Added protobuf map support.

2017-10-17 Thread Benjamin Bannier
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59987/#review188293 --- 3rdparty/stout/include/stout/protobuf.hpp Line 1 (original), 1

Re: Review Request 59987: Added protobuf map support.

2017-09-29 Thread Chun-Hung Hsiao
> On Sept. 22, 2017, 11:19 p.m., Chun-Hung Hsiao wrote: > > 3rdparty/stout/include/stout/protobuf.hpp > > Lines 433 (patched) > > > > > > The key can also be of an integral type. > > Qian Zhang wrote: > Yeah, I

Re: Review Request 59987: Added protobuf map support.

2017-09-29 Thread Chun-Hung Hsiao
> On Sept. 22, 2017, 11:14 p.m., Chun-Hung Hsiao wrote: > > 3rdparty/stout/include/stout/protobuf.hpp > > Lines 413 (patched) > > > > > > Not sure if it is cleaner to parse the map here because we're > >

Re: Review Request 59987: Added protobuf map support.

2017-09-29 Thread Qian Zhang
> On Sept. 23, 2017, 7:14 a.m., Chun-Hung Hsiao wrote: > > 3rdparty/stout/include/stout/protobuf.hpp > > Lines 413 (patched) > > > > > > Not sure if it is cleaner to parse the map here because we're > >

Re: Review Request 59987: Added protobuf map support.

2017-09-29 Thread Qian Zhang
> On Sept. 23, 2017, 7:19 a.m., Chun-Hung Hsiao wrote: > > 3rdparty/stout/include/stout/protobuf.hpp > > Lines 433 (patched) > > > > > > The key can also be of an integral type. Yeah, I had the same concern before.

Re: Review Request 59987: Added protobuf map support.

2017-09-22 Thread Chun-Hung Hsiao
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59987/#review186035 --- 3rdparty/stout/include/stout/protobuf.hpp Lines 433 (patched)

Re: Review Request 59987: Added protobuf map support.

2017-09-22 Thread Chun-Hung Hsiao
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59987/#review186031 --- 3rdparty/stout/include/stout/protobuf.hpp Lines 413 (patched)

Re: Review Request 59987: Added protobuf map support.

2017-07-03 Thread Qian Zhang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59987/ --- (Updated July 4, 2017, 10:43 a.m.) Review request for mesos, Anand Mazumdar

Re: Review Request 59987: Added protobuf map support.

2017-06-18 Thread Qian Zhang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59987/ --- (Updated June 18, 2017, 8:42 p.m.) Review request for mesos, Anand Mazumdar

Re: Review Request 59987: Added protobuf map support.

2017-06-18 Thread Qian Zhang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59987/ --- (Updated June 18, 2017, 8:39 p.m.) Review request for mesos, Anand Mazumdar

Re: Review Request 59987: Added protobuf map support.

2017-06-11 Thread Mesos Reviewbot Windows
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59987/#review177586 --- Patch looks great! Reviews applied: [59987] Passed command:

Review Request 59987: Added protobuf map support.

2017-06-11 Thread Qian Zhang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59987/ --- Review request for mesos, Anand Mazumdar and Zhitao Li. Repository: mesos