----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51336/#review146545 -----------------------------------------------------------
I'm not 100% convinced that moving scalar_base out of internal is justified by the inability to get doxygen to generate the correct docs. Can't we fix doxygen somehow? Why scalar_base but not value_base? they have the same function in the API structure. I think it may be correct to actually pull in avarything from the base classes with a using directive anyway to give the correct name lookup semantics (it matters if you have overloading). proton-c/bindings/cpp/include/proton/message_id.hpp (line 45) <https://reviews.apache.org/r/51336/#comment213050> This is an actual API change (even if it is a correct one). Can message_id really be empty? - Andrew Stitcher On Aug. 23, 2016, 4:02 p.m., Alan Conway wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/51336/ > ----------------------------------------------------------- > > (Updated Aug. 23, 2016, 4:02 p.m.) > > > Review request for qpid and Andrew Stitcher. > > > Repository: qpid-proton-git > > > Description > ------- > > Added empty() to scalar_base() > > Moved internal::scalar_base to proton::scalar_base. All it's public member > functions are part of the public API for proton::scalar, message_id and > annotation_key > so putting the class that contains them in the namespace internal makes that > unclear > and prevents doxygen from documenting them. > > Moved internal::value_base::empty() and type() to proton::value. There's no > need for them > to be on the internal class and they are public API. > > Note all scalar types need empty() because although the restricted types are > only allowed to be set as specific typesm, they can be missing from a message > which is represented by empty(). > > > Diffs > ----- > > proton-c/bindings/cpp/include/proton/annotation_key.hpp > c0c8ca6cd4d09a5d1720dd2ac8a16a71f91ab8b3 > proton-c/bindings/cpp/include/proton/codec/encoder.hpp > 847e0a9f29aba973f96fa06e3268a1bbe063cd53 > proton-c/bindings/cpp/include/proton/internal/scalar_base.hpp > 2e491d193d604d52b7ec91af510770fc9b4ac66e > proton-c/bindings/cpp/include/proton/message_id.hpp > 84f8ab0c28d5afa39e19a78040c0e07fd96b19b9 > proton-c/bindings/cpp/include/proton/scalar.hpp > ab7cb2a514d697c365cf568b4be7a842ae29aa3e > proton-c/bindings/cpp/include/proton/value.hpp > e7e9f7933038c1ba23b5afdbb102d59208e9ea4c > proton-c/bindings/cpp/src/encoder.cpp > e718c402a46d5423b70d178c1903b6ad767eeec5 > proton-c/bindings/cpp/src/scalar_base.cpp > c005122dedcfabe135bb62fe6c16eafe4da630cb > proton-c/bindings/cpp/src/value.cpp > c2232431f9f093e3530342ea5e3680a62737a4c8 > > Diff: https://reviews.apache.org/r/51336/diff/ > > > Testing > ------- > > ctest -R cpp on f23 clang/gcc 03/11, appveyor, travis > > > Thanks, > > Alan Conway > >
