> On Sept. 19, 2017, 7:39 p.m., Benjamin Bannier wrote: > > 3rdparty/stout/tests/json_tests.cpp > > Lines 358-359 (original) > > <https://reviews.apache.org/r/62160/diff/1/?file=1817622#file1817622line358> > > > > Since the code here was weird even before, it seems that we should > > change `JSON::True` and `JSON::False` to be factories of `JSON::Boolean`, > > e.g., > > > > Boolean False() > > { > > return {false}; > > } > > > > Boolean True() > > { > > return {true}; > > } > > > > With such factories we could keep users of `Boolean` as brief as they > > are now, and not resort to slicing as a usual measure. This change would > > also be in line with what we did when created `Bytest` factories out of > > existing subclasses. > > > > Would you be able to create a patch for that?
This will break all code that creates objects of type `JSON::True` or `JSON::False`. We could simultaneously fix all uses in mesos/libprocess, but technically `stout` is an independent library so I think we should not break API compatibility for such a small improvement. - Benno ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62160/#review185723 ----------------------------------------------------------- On Sept. 12, 2017, 11:08 a.m., Benno Evers wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/62160/ > ----------------------------------------------------------- > > (Updated Sept. 12, 2017, 11:08 a.m.) > > > Review request for mesos, Benjamin Bannier and Till Toenshoff. > > > Repository: mesos > > > Description > ------- > > Starting from Boost 1.62, Boost.Variant added additional > compile-time checks to its constructors to fix this > issue: https://svn.boost.org/trac10/ticket/11602 > > However, this breaks some places in stout which try > to access a derived class from a variant holding the > base class. > > > Diffs > ----- > > 3rdparty/stout/include/stout/json.hpp > 4f9ea1b34537026af80edd97fa0b3ae1a3dfa862 > 3rdparty/stout/include/stout/protobuf.hpp > 15690b66cc4ae0c1bf2c2176d73c385ca75d3c20 > 3rdparty/stout/tests/json_tests.cpp > adaa343faf3b557ad483675318b22eb90b1eeb52 > > > Diff: https://reviews.apache.org/r/62160/diff/1/ > > > Testing > ------- > > > Thanks, > > Benno Evers > >
