> On Nov. 21, 2013, 3:55 p.m., Andrew Stitcher wrote: > > I really don't like this approach very much: > > > > * I think that from the user point of view it is too easy to ignore the > > deprecation warnings. > > * The changes are somewhat clunky. > > > > I think it would be better to make application writers who really want to > > use the API turn it on explicitly with a #define. Perhaps called > > ALLOW_DEPRECATED_QMF_APIS. > > > > You can implement that with the following snippet at the head of every API > > file that is deprecated. > > > > Something like: > > > > #if !defined(ALLOW_DEPRECATED_QMF_APIS) && !defined(QMF_EXPORT) > > # error "In order to use the deprecated QMF Console API you need to define > > ALLOW_DEPRECATED_QMF_APIS" > > #endif > > > > This approach won't work well for deprecating individual API calls but in > > this case where we deprecate an entire API it is fine. > > > > (Thanks to Justin for the explicit allow suggestion)
Brilliant - yes this is a much better approach! I'm really uncomfortable with the proposed patch use of compiler-specific flags, and your approach would make things _very_ hard to ignore! I'll discard this review, and post a new one based on your suggestion. - Kenneth ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15747/#review29229 ----------------------------------------------------------- On Nov. 21, 2013, 1:19 p.m., Kenneth Giusti wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/15747/ > ----------------------------------------------------------- > > (Updated Nov. 21, 2013, 1:19 p.m.) > > > Review request for qpid, Andrew Stitcher and Ted Ross. > > > Bugs: qpid-5369 > https://issues.apache.org/jira/browse/qpid-5369 > > > Repository: qpid > > > Description > ------- > > Causes the compiler to issue deprecation warnings when the C++ Agent or > Console API classes are used by applications. Works for GNU C and Microsoft > Visual C++ only. > > Update: I'd like this change considered for 0.26 release, so please indicate > whether you approve this change on the jira > https://issues.apache.org/jira/browse/qpid-5369 so I can ping Justin for > approval. > > thanks > > > Diffs > ----- > > /trunk/qpid/cpp/bindings/qmf2/examples/cpp/CMakeLists.txt 1543123 > /trunk/qpid/cpp/bindings/qmf2/python/CMakeLists.txt 1543123 > /trunk/qpid/cpp/bindings/qmf2/ruby/CMakeLists.txt 1543123 > /trunk/qpid/cpp/include/qmf/Agent.h 1543123 > /trunk/qpid/cpp/include/qmf/AgentEvent.h 1543123 > /trunk/qpid/cpp/include/qmf/AgentSession.h 1543123 > /trunk/qpid/cpp/include/qmf/ConsoleEvent.h 1543123 > /trunk/qpid/cpp/include/qmf/ConsoleSession.h 1543123 > /trunk/qpid/cpp/include/qmf/Data.h 1543123 > /trunk/qpid/cpp/include/qmf/DataAddr.h 1543123 > /trunk/qpid/cpp/include/qmf/ImportExport.h 1543123 > /trunk/qpid/cpp/include/qmf/Query.h 1543123 > /trunk/qpid/cpp/include/qmf/Schema.h 1543123 > /trunk/qpid/cpp/include/qmf/SchemaId.h 1543123 > /trunk/qpid/cpp/include/qmf/SchemaMethod.h 1543123 > /trunk/qpid/cpp/include/qmf/SchemaProperty.h 1543123 > /trunk/qpid/cpp/include/qmf/Subscription.h 1543123 > /trunk/qpid/cpp/include/qmf/exceptions.h 1543123 > /trunk/qpid/cpp/include/qmf/posix/EventNotifier.h 1543123 > > Diff: https://reviews.apache.org/r/15747/diff/ > > > Testing > ------- > > Removed the warning suppression flags and built the Agent and Console > examples on linux and windows - validated that the proper deprecation > warnings were issued. > > > Thanks, > > Kenneth Giusti > >
