> 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. > > Alan Conway wrote: > 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? > > Andrew Stitcher wrote: > We've already decided this: > > The main examples will be C++11 with a separate C++03 directory with the > basic examples (about 4 I think, I don't remember exactly which). > (I think c++11 is sufficient although 14 would bring the convenience of > make_unique<>) > > We decided that mt is C++11 only, I don't think we decided this for > function injection. > > [Write this down, you seem to keep on forgetting we made this decision!]
Thanks, it is on the list of things I keep forgetting but I can't remember where I left it. > 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) > > Alan Conway wrote: > 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())` > > Andrew Stitcher wrote: > That would be completely unambiguous, but I think that double seconds is > pretty unambiguous tbh. `duration d(2.2); // duration of 2.2s`. Having the > operator*(double, duration) operator*(duration, double) is still a good idea. I considered that, but I think it is error-prone if duration(1) and duration(1.0) mean different things. Another (less explicit) solution is to have `const int64_t duration::SECOND` instead of the constants being `duration`: then all the right conversions happen implicitly with the normal C++ arithmetic promotion and conversion rules, no need for extra ctor or ops. Explicit is probably better though. > 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. > > Alan Conway wrote: > See examples question above > > Andrew Stitcher wrote: > No, this is in a shared header file you *must* put a feature macro in > here. Whether the examples are 03 or 11 doesn't matter. Agreed, over-pasting. > 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? > > Alan Conway wrote: > 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. Got another vote for fixed frequency so I'll update. - 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 > >
