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/03 19:50:33 UTC
Review Request 18700: JSON parsing and equality.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18700/
-----------------------------------------------------------
Review request for mesos, Ben Mahler and Vinod Kone.
Repository: mesos-git
Description
-------
See summary.
Diffs
-----
3rdparty/libprocess/3rdparty/stout/include/stout/json.hpp 778398af0e166709a777d770b7fddda309a60add
3rdparty/libprocess/3rdparty/stout/tests/json_tests.cpp 5b7cbb9b42aaa0dc4f3e732d4ceae496bcb15ea6
Diff: https://reviews.apache.org/r/18700/diff/
Testing
-------
make check
Thanks,
Benjamin Hindman
Re: Review Request 18700: JSON parsing and equality.
Posted by Benjamin Hindman <be...@berkeley.edu>.
> On March 3, 2014, 6:58 p.m., Dominic Hamon wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/json.hpp, line 168
> > <https://reviews.apache.org/r/18700/diff/1/?file=508464#file508464line168>
> >
> > why not dynamic_cast<T> for here and as()?
Unless I'm missing something that's not how boost variant is implemented and you're supposed to use boost::get.
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18700/#review36004
-----------------------------------------------------------
On March 3, 2014, 6:50 p.m., Benjamin Hindman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18700/
> -----------------------------------------------------------
>
> (Updated March 3, 2014, 6:50 p.m.)
>
>
> Review request for mesos, Ben Mahler and Vinod Kone.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/include/stout/json.hpp 778398af0e166709a777d770b7fddda309a60add
> 3rdparty/libprocess/3rdparty/stout/tests/json_tests.cpp 5b7cbb9b42aaa0dc4f3e732d4ceae496bcb15ea6
>
> Diff: https://reviews.apache.org/r/18700/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Benjamin Hindman
>
>
Re: Review Request 18700: JSON parsing and equality.
Posted by Dominic Hamon <dh...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18700/#review36004
-----------------------------------------------------------
3rdparty/libprocess/3rdparty/stout/include/stout/json.hpp
<https://reviews.apache.org/r/18700/#comment66796>
why not dynamic_cast<T> for here and as()?
- Dominic Hamon
On March 3, 2014, 10:50 a.m., Benjamin Hindman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18700/
> -----------------------------------------------------------
>
> (Updated March 3, 2014, 10:50 a.m.)
>
>
> Review request for mesos, Ben Mahler and Vinod Kone.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/include/stout/json.hpp 778398af0e166709a777d770b7fddda309a60add
> 3rdparty/libprocess/3rdparty/stout/tests/json_tests.cpp 5b7cbb9b42aaa0dc4f3e732d4ceae496bcb15ea6
>
> Diff: https://reviews.apache.org/r/18700/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Benjamin Hindman
>
>
Re: Review Request 18700: JSON parsing and equality.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18700/#review36045
-----------------------------------------------------------
Ship it!
Ship It!
- Vinod Kone
On March 3, 2014, 6:50 p.m., Benjamin Hindman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18700/
> -----------------------------------------------------------
>
> (Updated March 3, 2014, 6:50 p.m.)
>
>
> Review request for mesos, Ben Mahler and Vinod Kone.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/include/stout/json.hpp 778398af0e166709a777d770b7fddda309a60add
> 3rdparty/libprocess/3rdparty/stout/tests/json_tests.cpp 5b7cbb9b42aaa0dc4f3e732d4ceae496bcb15ea6
>
> Diff: https://reviews.apache.org/r/18700/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Benjamin Hindman
>
>
Re: Review Request 18700: JSON parsing and equality.
Posted by Benjamin Hindman <be...@berkeley.edu>.
> On March 4, 2014, midnight, Charlie Carson wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/json.hpp, line 352
> > <https://reviews.apache.org/r/18700/diff/1/?file=508464#file508464line352>
> >
> > this CR reads like picoJSON is an implementation detail of stout::JSON - is that the long term plan? or do we want to switch to picoJSON and remove stout::JSON eventually?
> >
> > if it's the later, then you should probably make these guys public so calling code can switch over to picoJSON incrementally
We're going to keep a level of indirection for now. There are things that PicoJSON doesn't support that we'll be adding (like JSON::Object::find, see an upcoming review) so it's nice to have a layer of indirection. That and if in the future we decide that PicoJSON parsing (or serializing, if we use that) is too slow we can easily swap it for something like yajl or RapidJSON.
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18700/#review36057
-----------------------------------------------------------
On March 3, 2014, 6:50 p.m., Benjamin Hindman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18700/
> -----------------------------------------------------------
>
> (Updated March 3, 2014, 6:50 p.m.)
>
>
> Review request for mesos, Ben Mahler and Vinod Kone.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/include/stout/json.hpp 778398af0e166709a777d770b7fddda309a60add
> 3rdparty/libprocess/3rdparty/stout/tests/json_tests.cpp 5b7cbb9b42aaa0dc4f3e732d4ceae496bcb15ea6
>
> Diff: https://reviews.apache.org/r/18700/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Benjamin Hindman
>
>
Re: Review Request 18700: JSON parsing and equality.
Posted by Charlie Carson <ch...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18700/#review36057
-----------------------------------------------------------
Ship it!
Ship It!
3rdparty/libprocess/3rdparty/stout/include/stout/json.hpp
<https://reviews.apache.org/r/18700/#comment66887>
this CR reads like picoJSON is an implementation detail of stout::JSON - is that the long term plan? or do we want to switch to picoJSON and remove stout::JSON eventually?
if it's the later, then you should probably make these guys public so calling code can switch over to picoJSON incrementally
- Charlie Carson
On March 3, 2014, 6:50 p.m., Benjamin Hindman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18700/
> -----------------------------------------------------------
>
> (Updated March 3, 2014, 6:50 p.m.)
>
>
> Review request for mesos, Ben Mahler and Vinod Kone.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/include/stout/json.hpp 778398af0e166709a777d770b7fddda309a60add
> 3rdparty/libprocess/3rdparty/stout/tests/json_tests.cpp 5b7cbb9b42aaa0dc4f3e732d4ceae496bcb15ea6
>
> Diff: https://reviews.apache.org/r/18700/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Benjamin Hindman
>
>