You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Michael Park <mp...@apache.org> on 2017/01/07 10:02:08 UTC
Review Request 55296: Used `jsonify` in `operator<<` for `JSON::*` to
reduce duplicate code.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55296/
-----------------------------------------------------------
Review request for mesos, Alexander Rojas and Joris Van Remoortere.
Repository: mesos
Description
-------
The printing logic for `json.hpp` and `jsonify.hpp` are currently duplicated.
We can reduce this duplication by leveraging `jsonify` for the implementation of `operator<<` for `JSON::*`.
Since `JSON::Value`s are not generally used for printing, there should be no performance concerns here.
Diffs
-----
3rdparty/stout/include/stout/json.hpp 62ce15274677112d142a3c829b4a9f06258c9e2c
Diff: https://reviews.apache.org/r/55296/diff/
Testing
-------
Thanks,
Michael Park
Re: Review Request 55296: Used `jsonify` in `operator<<` for
`JSON::*` to reduce duplicate code.
Posted by Michael Park <mp...@apache.org>.
> On Jan. 9, 2017, 8:52 a.m., Alexander Rojas wrote:
> > 3rdparty/stout/include/stout/json.hpp, line 703
> > <https://reviews.apache.org/r/55296/diff/1/?file=1599425#file1599425line703>
> >
> > This needs either UNREACHABLE() or to return stream to avoid warnings.
There seems to be some issue with the git repo and reviewboard or something...
https://mesos.slack.com/archives/dev/p1483997930001234
I'll update this as soon as it's resolved.
- Michael
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55296/#review160902
-----------------------------------------------------------
On Jan. 7, 2017, 2:02 a.m., Michael Park wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55296/
> -----------------------------------------------------------
>
> (Updated Jan. 7, 2017, 2:02 a.m.)
>
>
> Review request for mesos, Alexander Rojas and Joris Van Remoortere.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The printing logic for `json.hpp` and `jsonify.hpp` are currently duplicated.
> We can reduce this duplication by leveraging `jsonify` for the implementation of `operator<<` for `JSON::*`.
> Since `JSON::Value`s are not generally used for printing, there should be no performance concerns here.
>
>
> Diffs
> -----
>
> 3rdparty/stout/include/stout/json.hpp 62ce15274677112d142a3c829b4a9f06258c9e2c
>
> Diff: https://reviews.apache.org/r/55296/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Michael Park
>
>
Re: Review Request 55296: Used `jsonify` in `operator<<` for
`JSON::*` to reduce duplicate code.
Posted by Alexander Rojas <al...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55296/#review160902
-----------------------------------------------------------
3rdparty/stout/include/stout/json.hpp (line 679)
<https://reviews.apache.org/r/55296/#comment232146>
This needs either UNREACHABLE() or to return stream to avoid warnings.
- Alexander Rojas
On Jan. 7, 2017, 11:02 a.m., Michael Park wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55296/
> -----------------------------------------------------------
>
> (Updated Jan. 7, 2017, 11:02 a.m.)
>
>
> Review request for mesos, Alexander Rojas and Joris Van Remoortere.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The printing logic for `json.hpp` and `jsonify.hpp` are currently duplicated.
> We can reduce this duplication by leveraging `jsonify` for the implementation of `operator<<` for `JSON::*`.
> Since `JSON::Value`s are not generally used for printing, there should be no performance concerns here.
>
>
> Diffs
> -----
>
> 3rdparty/stout/include/stout/json.hpp 62ce15274677112d142a3c829b4a9f06258c9e2c
>
> Diff: https://reviews.apache.org/r/55296/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Michael Park
>
>
Re: Review Request 55296: Used `jsonify` in `operator<<` for
`JSON::*` to reduce duplicate code.
Posted by Alexander Rojas <al...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55296/#review161047
-----------------------------------------------------------
Ship it!
Ship It!
- Alexander Rojas
On Jan. 10, 2017, 1:36 a.m., Michael Park wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55296/
> -----------------------------------------------------------
>
> (Updated Jan. 10, 2017, 1:36 a.m.)
>
>
> Review request for mesos, Alexander Rojas and Joris Van Remoortere.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The printing logic for `json.hpp` and `jsonify.hpp` are currently
> duplicated. We can reduce this duplication by leveraging `jsonify` for
> the implementation of `operator<<` for `JSON::*`. Since `JSON::Value`s
> are not generally used for printing, there should be no performance
> concerns here.
>
>
> Diffs
> -----
>
> 3rdparty/stout/include/stout/json.hpp 62ce15274677112d142a3c829b4a9f06258c9e2c
>
> Diff: https://reviews.apache.org/r/55296/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Michael Park
>
>
Re: Review Request 55296: Used `jsonify` in `operator<<` for
`JSON::*` to reduce duplicate code.
Posted by Michael Park <mp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55296/
-----------------------------------------------------------
(Updated Jan. 10, 2017, 3:22 p.m.)
Review request for mesos, Alexander Rojas and Joris Van Remoortere.
Bugs: MESOS-6349
https://issues.apache.org/jira/browse/MESOS-6349
Repository: mesos
Description
-------
The printing logic for `json.hpp` and `jsonify.hpp` are currently
duplicated. We can reduce this duplication by leveraging `jsonify` for
the implementation of `operator<<` for `JSON::*`. Since `JSON::Value`s
are not generally used for printing, there should be no performance
concerns here.
Diffs
-----
3rdparty/stout/include/stout/json.hpp 62ce15274677112d142a3c829b4a9f06258c9e2c
Diff: https://reviews.apache.org/r/55296/diff/
Testing
-------
Thanks,
Michael Park
Re: Review Request 55296: Used `jsonify` in `operator<<` for
`JSON::*` to reduce duplicate code.
Posted by Michael Park <mp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55296/
-----------------------------------------------------------
(Updated Jan. 9, 2017, 4:36 p.m.)
Review request for mesos, Alexander Rojas and Joris Van Remoortere.
Changes
-------
Addressed arojas' comment.
Repository: mesos
Description (updated)
-------
The printing logic for `json.hpp` and `jsonify.hpp` are currently
duplicated. We can reduce this duplication by leveraging `jsonify` for
the implementation of `operator<<` for `JSON::*`. Since `JSON::Value`s
are not generally used for printing, there should be no performance
concerns here.
Diffs (updated)
-----
3rdparty/stout/include/stout/json.hpp 62ce15274677112d142a3c829b4a9f06258c9e2c
Diff: https://reviews.apache.org/r/55296/diff/
Testing
-------
Thanks,
Michael Park