mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benjamin Hindman <b...@berkeley.edu>
Subject Re: Review Request 60035: Added support for printing JSON values via `jsonify`.
Date Wed, 14 Jun 2017 01:41:12 GMT

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


Fix it, then Ship it!





3rdparty/stout/include/stout/json.hpp
Line 668 (original), 668 (patched)
<https://reviews.apache.org/r/60035/#comment251567>

    Can we comment here why we need to implement these `json` overloads?



3rdparty/stout/include/stout/json.hpp
Lines 681 (patched)
<https://reviews.apache.org/r/60035/#comment251557>

    `{` on newline please.



3rdparty/stout/include/stout/json.hpp
Lines 708 (patched)
<https://reviews.apache.org/r/60035/#comment251571>

    Small comment here as to why we do nothinig?



3rdparty/stout/include/stout/json.hpp
Lines 711 (patched)
<https://reviews.apache.org/r/60035/#comment251568>

    Move into `json` below and make explicit?



3rdparty/stout/include/stout/json.hpp
Lines 722 (patched)
<https://reviews.apache.org/r/60035/#comment251570>

    I think we need more comments here as to why we need to have this special versin of `json`
in the first place.



3rdparty/stout/include/stout/json.hpp
Lines 758 (patched)
<https://reviews.apache.org/r/60035/#comment251569>

    Doesn't this patch fix these TODOs?


- Benjamin Hindman


On June 13, 2017, 8:24 p.m., Michael Park wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60035/
> -----------------------------------------------------------
> 
> (Updated June 13, 2017, 8:24 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added support for printing JSON values via `jsonify`.
> 
> 
> Diffs
> -----
> 
>   3rdparty/stout/include/stout/json.hpp aa581c0345679b6f9617fb986d744110a728f042 
>   3rdparty/stout/include/stout/jsonify.hpp 7d84bac18abbe9f5bcf67a377d4e742e31b4c94d 
>   3rdparty/stout/tests/jsonify_tests.cpp d06eba31e6b7f252b120a10066e4d7c9a25ce4b5 
> 
> 
> Diff: https://reviews.apache.org/r/60035/diff/3/
> 
> 
> Testing
> -------
> 
> Added new tests + `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message