> On June 7, 2016, 9:40 p.m., Andrew Stitcher wrote: > > You need to compile this code with C++03 and fix up the places that end up > > needing a feature macro to avoid breakage.
Yep, known limitation. I need to add C++03 callback support. This is a first cut in C++11 without worrying about that. All your comments will apply to the next cut. Quetion: How to organize examples for features that are used differntly in C++03 and C++11? - separate c++11/c++03 examples? - conditionalized examples that work under both, with different example code chunks selected by #ifdef? Clean C++11 examples would be nice. They have to be defined against a specific standard: 11, 14 or 17. We could choose a base standard (11) and use conditional macros for features of 14/17 but I think we should stick to clean c++11 - these are proton examples not C++ tutorials, if we can't write them in C++11 we should rethink the API. That means separate examples (with some redundancy) are needed at least for features that can't be used the same way in C++03 (schedule, inject) "Universal" conditionalized examples based on 03, with fake_cpp11.hpp, #if PN_CPP_HAS_STD_FUNCTION etc. : easier for us to maintain, maybe easier for users transitioning from 03 to 11 (side-by-side code) but cluttered for users that are on 11. On the other hand very few of the examples actually *need* C++11 features, so if we do have users stuck on older compilers it's a bit obnoxious to make the base examples uncompilable when they could easily be portable. It really hinges on how many users are in that position. A hybrid of the above: conditionalized examples where C++11 and 03 are nearly the same (basically what we have now) and separate examples where they diverge. I'm not sure, thoughts? > On June 7, 2016, 9:40 p.m., Andrew Stitcher wrote: > > examples/cpp/scheduled_send.cpp, line 29 > > <https://reviews.apache.org/r/48362/diff/1/?file=1410589#file1410589line29> > > > > This is C++11 only don't need "fake_cpp11.hpp", and can just use > > "override" directly. See examples question above > On June 7, 2016, 9:40 p.m., Andrew Stitcher wrote: > > examples/cpp/scheduled_send.cpp, line 54 > > <https://reviews.apache.org/r/48362/diff/1/?file=1410589#file1410589line54> > > > > Should be able to combine into a single line with no loss of > > readability (IMO): > > One common style seems to be - > > > > ``` > > c.schedule(timeout, [this]() { > > this->cancel(); > > }); > > ``` See examples question above. > On June 7, 2016, 9:40 p.m., Andrew Stitcher wrote: > > examples/cpp/scheduled_send.cpp, line 89 > > <https://reviews.apache.org/r/48362/diff/1/?file=1410589#file1410589line89> > > > > Should the delay here take account of the actual now time? > > > > In other words, are we trying for a fixed frequency or an "at least" > > delay? For purposes of showing how to use schedule() I don't think it matters - I'll add a comment to clarify this this example is doing a delay *between* sends. Making it fixed frequency would allow us to show the use of now(), so we could do that too. > On June 7, 2016, 9:40 p.m., Andrew Stitcher wrote: > > proton-c/bindings/cpp/include/proton/container.hpp, line 201 > > <https://reviews.apache.org/r/48362/diff/1/?file=1410590#file1410590line201> > > > > Need to add a new feature flag I'll need to look up the actual name, > > but PN_CPP_HAS_STD_FUNCTION will do for now. See examples question above > On June 7, 2016, 9:40 p.m., Andrew Stitcher wrote: > > examples/cpp/scheduled_send.cpp, line 44 > > <https://reviews.apache.org/r/48362/diff/1/?file=1410589#file1410589line44> > > > > Yep that's a good idea, lets add double constructors to duration (just > > not as pzrt of this change) Agreed, will restrain myself :) I think we need double `op*` rather than constructors to keep the units unambiguous: `duration d(2.0*duration::SECOND)`. A double ctor will still have you doing `duration d (2.0*duration::SECOND.milliseconds())` - Alan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48362/#review136551 ----------------------------------------------------------- On June 7, 2016, 9:23 p.m., Alan Conway wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/48362/ > ----------------------------------------------------------- > > (Updated June 7, 2016, 9:23 p.m.) > > > Review request for qpid, Andrew Stitcher, Cliff Jansen, and Justin Ross. > > > Repository: qpid-proton-git > > > Description > ------- > > PROTON-1221: c++ container::schedule() support. > > > Diffs > ----- > > examples/cpp/CMakeLists.txt 06ec1a49c00a98c3a602c9b6e76bf311345c397b > examples/cpp/README.dox 897b302da0c0c132093b5f9977d28ac274cfeea9 > examples/cpp/mt/epoll_container.cpp > feea3007a8b732a47177a50d6ff385f9d60f1a63 > examples/cpp/scheduled_send.cpp PRE-CREATION > proton-c/bindings/cpp/include/proton/container.hpp > 9665346cef187fccf59049ea4f5ce927dd46b60b > proton-c/bindings/cpp/include/proton/default_container.hpp > f50869cb28b9d94784ab3329dcec43085eab4393 > proton-c/bindings/cpp/src/container_impl.hpp > 8fd64d00c4191c34ec3e6dd5da32c121f8ad2718 > proton-c/bindings/cpp/src/container_impl.cpp > ce6a2b2a097795d49f40e4a962e91db9222588f0 > proton-c/bindings/cpp/src/test_dummy_container.hpp > 730790140bea671796032b4daa95492ed1c17c25 > > Diff: https://reviews.apache.org/r/48362/diff/ > > > Testing > ------- > > > Thanks, > > Alan Conway > >
