Re: Review Request 38342: Add JSON::protobuf for google::protobuf::RepeatedPtrField (stout part)

2015-11-04 Thread Klaus Ma
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38342/ --- (Updated Nov. 4, 2015, 9:19 p.m.) Review request for mesos, Alexander

Re: Review Request 38342: Add JSON::protobuf for google::protobuf::RepeatedPtrField (stout part)

2015-11-02 Thread Michael Park
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38342/#review104848 --- Ship it! Ship It! - Michael Park On Oct. 20, 2015, 2:39 a.m.,

Re: Review Request 38342: Add JSON::protobuf for google::protobuf::RepeatedPtrField (stout part)

2015-10-19 Thread Michael Park
> On Oct. 18, 2015, 11:02 p.m., Michael Park wrote: > > 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp, lines 182-189 > > > > > > Indent 2 spaces, not the strings themselves, but within the strings. >

Re: Review Request 38342: Add JSON::protobuf for google::protobuf::RepeatedPtrField (stout part)

2015-10-19 Thread Michael Park
> On Oct. 18, 2015, 11:02 p.m., Michael Park wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp, lines 764-768 > > > > > > As mentioned before, let's remove the `default` case entirely. Refer

Re: Review Request 38342: Add JSON::protobuf for google::protobuf::RepeatedPtrField (stout part)

2015-10-19 Thread Klaus Ma
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38342/#review103214 --- The code diff of #38335 are alos upedted based on master. - Klaus

Re: Review Request 38342: Add JSON::protobuf for google::protobuf::RepeatedPtrField (stout part)

2015-10-19 Thread Klaus Ma
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38342/ --- (Updated Oct. 20, 2015, 2:39 a.m.) Review request for mesos, Alexander

Re: Review Request 38342: Add JSON::protobuf for google::protobuf::RepeatedPtrField (stout part)

2015-10-18 Thread Klaus Ma
> On Oct. 18, 2015, 11:02 p.m., Michael Park wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp, lines 764-768 > > > > > > As mentioned before, let's remove the `default` case entirely. Refer

Re: Review Request 38342: Add JSON::protobuf for google::protobuf::RepeatedPtrField (stout part)

2015-10-18 Thread Michael Park
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38342/#review103078 --- 3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp

Re: Review Request 38342: Add JSON::protobuf for google::protobuf::RepeatedPtrField (stout part)

2015-10-16 Thread Klaus Ma
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38342/ --- (Updated Oct. 16, 2015, 9:56 a.m.) Review request for mesos, Alexander

Re: Review Request 38342: Add JSON::protobuf for google::protobuf::RepeatedPtrField (stout part)

2015-10-12 Thread Alexander Rukletsov
> On Sept. 29, 2015, 10:43 a.m., Alexander Rukletsov wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp, lines 773-774 > > > > > > Could you please help me understand why this check is

Re: Review Request 38342: Add JSON::protobuf for google::protobuf::RepeatedPtrField (stout part)

2015-10-12 Thread Alexander Rukletsov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38342/#review102344 --- Ship it!

Re: Review Request 38342: Add JSON::protobuf for google::protobuf::RepeatedPtrField (stout part)

2015-10-12 Thread Alexander Rukletsov
> On Sept. 29, 2015, 12:14 p.m., Alexander Rukletsov wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp, lines 762-763 > > > > > > I've seen your discussion with @Jan above, here is what I

Re: Review Request 38342: Add JSON::protobuf for google::protobuf::RepeatedPtrField (stout part)

2015-10-06 Thread Klaus Ma
> On Sept. 29, 2015, 4:17 p.m., Alexander Rukletsov wrote: > > One high level suggestion. > > > > After looking at our http code, I realized that we use the same pattern > > again and again, for example: > > ``` > > JSON::Array array; > >

Re: Review Request 38342: Add JSON::protobuf for google::protobuf::RepeatedPtrField (stout part)

2015-10-05 Thread Michael Park
> On Sept. 28, 2015, 9:11 a.m., Jan Schlicht wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp, line 762 > > > > > > Because this code has been changed from a constructor to a function, we >

Re: Review Request 38342: Add JSON::protobuf for google::protobuf::RepeatedPtrField (stout part)

2015-10-05 Thread Michael Park
> On Sept. 29, 2015, 4:17 p.m., Alexander Rukletsov wrote: > > One high level suggestion. > > > > After looking at our http code, I realized that we use the same pattern > > again and again, for example: > > ``` > > JSON::Array array; > >

Re: Review Request 38342: Add JSON::protobuf for google::protobuf::RepeatedPtrField (stout part)

2015-10-05 Thread Alexander Rukletsov
> On Sept. 29, 2015, 4:17 p.m., Alexander Rukletsov wrote: > > One high level suggestion. > > > > After looking at our http code, I realized that we use the same pattern > > again and again, for example: > > ``` > > JSON::Array array; > >

Re: Review Request 38342: Add JSON::protobuf for google::protobuf::RepeatedPtrField (stout part)

2015-10-05 Thread Jan Schlicht
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38342/#review101466 --- Ship it! Regarding the "`ABORT` or `Try<>`" discussion, I agree

Re: Review Request 38342: Add JSON::protobuf for google::protobuf::RepeatedPtrField (stout part)

2015-10-05 Thread Alexander Rukletsov
> On Sept. 29, 2015, 10:43 a.m., Alexander Rukletsov wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp, lines 773-774 > > > > > > Could you please help me understand why this check is

Re: Review Request 38342: Add JSON::protobuf for google::protobuf::RepeatedPtrField (stout part)

2015-10-04 Thread Klaus Ma
> On Sept. 29, 2015, 4:17 p.m., Alexander Rukletsov wrote: > > One high level suggestion. > > > > After looking at our http code, I realized that we use the same pattern > > again and again, for example: > > ``` > > JSON::Array array; > >

Re: Review Request 38342: Add JSON::protobuf for google::protobuf::RepeatedPtrField (stout part)

2015-10-04 Thread Klaus Ma
> On Sept. 29, 2015, 10:43 a.m., Alexander Rukletsov wrote: > > 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp, line 166 > > > > > > See my comment above. I think you should not repeat testing

Re: Review Request 38342: Add JSON::protobuf for google::protobuf::RepeatedPtrField (stout part)

2015-10-04 Thread Klaus Ma
> On Sept. 29, 2015, 4:17 p.m., Alexander Rukletsov wrote: > > One high level suggestion. > > > > After looking at our http code, I realized that we use the same pattern > > again and again, for example: > > ``` > > JSON::Array array; > >

Re: Review Request 38342: Add JSON::protobuf for google::protobuf::RepeatedPtrField (stout part)

2015-10-04 Thread Klaus Ma
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38342/ --- (Updated Oct. 4, 2015, 11:29 a.m.) Review request for mesos, Alexander

Re: Review Request 38342: Add JSON::protobuf for google::protobuf::RepeatedPtrField (stout part)

2015-09-30 Thread Alexander Rukletsov
> On Sept. 29, 2015, 4:17 p.m., Alexander Rukletsov wrote: > > One high level suggestion. > > > > After looking at our http code, I realized that we use the same pattern > > again and again, for example: > > ``` > > JSON::Array array; > >

Re: Review Request 38342: Add JSON::protobuf for google::protobuf::RepeatedPtrField (stout part)

2015-09-30 Thread Michael Park
> On Sept. 29, 2015, 4:17 p.m., Alexander Rukletsov wrote: > > One high level suggestion. > > > > After looking at our http code, I realized that we use the same pattern > > again and again, for example: > > ``` > > JSON::Array array; > >

Re: Review Request 38342: Add JSON::protobuf for google::protobuf::RepeatedPtrField (stout part)

2015-09-29 Thread Alexander Rukletsov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38342/#review100953 --- 3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp

Re: Review Request 38342: Add JSON::protobuf for google::protobuf::RepeatedPtrField (stout part)

2015-09-29 Thread Alexander Rukletsov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38342/#review100962 --- 3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp

Re: Review Request 38342: Add JSON::protobuf for google::protobuf::RepeatedPtrField (stout part)

2015-09-29 Thread Alexander Rukletsov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38342/#review100964 --- One high level suggestion. After looking at our http code, I

Re: Review Request 38342: Add JSON::protobuf for google::protobuf::RepeatedPtrField (stout part)

2015-09-29 Thread Klaus Ma
> On Sept. 29, 2015, 10:43 a.m., Alexander Rukletsov wrote: > > 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.h, line 40 > > > > > > Just to confirm: this file is generated using bundled protobuf, right?

Re: Review Request 38342: Add JSON::protobuf for google::protobuf::RepeatedPtrField (stout part)

2015-09-28 Thread Klaus Ma
> On Sept. 28, 2015, 9:11 a.m., Jan Schlicht wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp, line 762 > > > > > > Because this code has been changed from a constructor to a function, we >

Re: Review Request 38342: Add JSON::protobuf for google::protobuf::RepeatedPtrField (stout part)

2015-09-28 Thread Jan Schlicht
> On Sept. 25, 2015, 3:35 p.m., Alexander Rukletsov wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp, line 609 > > > > > > 1. You can also use `inline`, that's what we usually do. > > 2.

Re: Review Request 38342: Add JSON::protobuf for google::protobuf::RepeatedPtrField (stout part)

2015-09-28 Thread Klaus Ma
> On Sept. 28, 2015, 9:11 a.m., Jan Schlicht wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp, line 762 > > > > > > Because this code has been changed from a constructor to a function, we >

Re: Review Request 38342: Add JSON::protobuf for google::protobuf::RepeatedPtrField (stout part)

2015-09-28 Thread Jan Schlicht
> On Sept. 28, 2015, 11:11 a.m., Jan Schlicht wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp, line 762 > > > > > > Because this code has been changed from a constructor to a function, we >

Re: Review Request 38342: Add JSON::protobuf for google::protobuf::RepeatedPtrField (stout part)

2015-09-26 Thread Klaus Ma
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38342/ --- (Updated Sept. 27, 2015, 1:34 a.m.) Review request for mesos, Alexander

Re: Review Request 38342: Add JSON::protobuf for google::protobuf::RepeatedPtrField (stout part)

2015-09-25 Thread Klaus Ma
> On Sept. 25, 2015, 1:35 p.m., Alexander Rukletsov wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp, line 609 > > > > > > 1. You can also use `inline`, that's what we usually do. > > 2.

Re: Review Request 38342: Add JSON::protobuf for google::protobuf::RepeatedPtrField (stout part)

2015-09-25 Thread Alexander Rukletsov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38342/#review100575 --- 3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp

Re: Review Request 38342: Add JSON::protobuf for google::protobuf::RepeatedPtrField (stout part)

2015-09-24 Thread Klaus Ma
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38342/ --- (Updated Sept. 24, 2015, 2:37 p.m.) Review request for mesos, Alexander

Re: Review Request 38342: Add JSON::protobuf for google::protobuf::RepeatedPtrField (stout part)

2015-09-24 Thread Klaus Ma
> On Sept. 15, 2015, 12:59 p.m., Jan Schlicht wrote: > > Please rebase your patch, latest commits in master lots of JSON related > > stuff. > > Jan Schlicht wrote: > Will review this RR after rebase. The code diff was uploaded after resolving the conflict - Klaus

Re: Review Request 38342: Add JSON::protobuf for google::protobuf::RepeatedPtrField (stout part)

2015-09-24 Thread Jan Schlicht
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38342/#review100413 --- 3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp

Re: Review Request 38342: Add JSON::protobuf for google::protobuf::RepeatedPtrField (stout part)

2015-09-24 Thread Klaus Ma
> On Sept. 24, 2015, 3:52 p.m., Jan Schlicht wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp, line 609 > > > > > > s/protobuf(const T& message)/protobuf(const google::protobuf::Message& > >

Re: Review Request 38342: Add JSON::protobuf for google::protobuf::RepeatedPtrField (stout part)

2015-09-15 Thread Klaus Ma
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38342/ --- (Updated Sept. 15, 2015, 9:43 a.m.) Review request for mesos, Alexander

Re: Review Request 38342: Add JSON::protobuf for google::protobuf::RepeatedPtrField (stout part)

2015-09-15 Thread Klaus Ma
> On Sept. 14, 2015, 5:39 p.m., Alexander Rukletsov wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/json.hpp, line 112 > > > > > > Let's `reserve()` first. Addressed, thanks :). - Klaus

Re: Review Request 38342: Add JSON::protobuf for google::protobuf::RepeatedPtrField (stout part)

2015-09-15 Thread Jan Schlicht
> On Sept. 15, 2015, 2:59 p.m., Jan Schlicht wrote: > > Please rebase your patch, latest commits in master lots of JSON related > > stuff. Will review this RR after rebase. - Jan --- This is an automatically generated e-mail. To

Re: Review Request 38342: Add JSON::protobuf for google::protobuf::RepeatedPtrField (stout part)

2015-09-15 Thread Jan Schlicht
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38342/#review99020 --- Please rebase your patch, latest commits in master lots of JSON