You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Benjamin Hindman <be...@berkeley.edu> on 2014/03/11 02:48:22 UTC

Review Request 19003: Fixed JSON to protobuf parsing for enums.

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

Review request for mesos and Ben Mahler.


Repository: mesos-git


Description
-------

See summary.


Diffs
-----

  3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp 6fe795ff0a0b5d91e63ece64cd9ba6e499fca912 

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


Testing
-------

make check


Thanks,

Benjamin Hindman


Re: Review Request 19003: Fixed JSON to protobuf parsing for enums.

Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19003/#review36749
-----------------------------------------------------------


Patch looks great!

Reviews applied: [19003]

All tests passed.

- Mesos ReviewBot


On March 11, 2014, 1:48 a.m., Benjamin Hindman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19003/
> -----------------------------------------------------------
> 
> (Updated March 11, 2014, 1:48 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp 6fe795ff0a0b5d91e63ece64cd9ba6e499fca912 
> 
> Diff: https://reviews.apache.org/r/19003/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Hindman
> 
>


Re: Review Request 19003: Fixed JSON to protobuf parsing for enums.

Posted by Benjamin Hindman <be...@berkeley.edu>.

> On March 11, 2014, 4:01 a.m., Ben Mahler wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp, line 339
> > <https://reviews.apache.org/r/19003/diff/1/?file=515853#file515853line339>
> >
> >     Can we first enforce that the double represents a whole number? If it's not a whole number an Error would be nice.

Thinking more about this case, I decided to drop it because:

(1) It will allow someone to use the value '4.0' which is rather unfortunate.
(2) I'd rather favor (force?) the string version of the enum for readability in the JSON.


- Benjamin


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


On March 11, 2014, 1:48 a.m., Benjamin Hindman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19003/
> -----------------------------------------------------------
> 
> (Updated March 11, 2014, 1:48 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp 6fe795ff0a0b5d91e63ece64cd9ba6e499fca912 
> 
> Diff: https://reviews.apache.org/r/19003/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Hindman
> 
>


Re: Review Request 19003: Fixed JSON to protobuf parsing for enums.

Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19003/#review36754
-----------------------------------------------------------

Ship it!


Modulo issue below.


3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp
<https://reviews.apache.org/r/19003/#comment67940>

    Can we first enforce that the double represents a whole number? If it's not a whole number an Error would be nice.


- Ben Mahler


On March 11, 2014, 1:48 a.m., Benjamin Hindman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19003/
> -----------------------------------------------------------
> 
> (Updated March 11, 2014, 1:48 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp 6fe795ff0a0b5d91e63ece64cd9ba6e499fca912 
> 
> Diff: https://reviews.apache.org/r/19003/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Hindman
> 
>


Re: Review Request 19003: Fixed JSON to protobuf parsing for enums.

Posted by Dominic Hamon <dh...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19003/#review36802
-----------------------------------------------------------



3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp
<https://reviews.apache.org/r/19003/#comment67996>

    a nit: can we avoid using std types as variable names?


- Dominic Hamon


On March 10, 2014, 6:48 p.m., Benjamin Hindman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19003/
> -----------------------------------------------------------
> 
> (Updated March 10, 2014, 6:48 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp 6fe795ff0a0b5d91e63ece64cd9ba6e499fca912 
> 
> Diff: https://reviews.apache.org/r/19003/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Hindman
> 
>