You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@qpid.apache.org by Kenneth Giusti <kg...@apache.org> on 2013/11/21 03:44:05 UTC
Review Request 15747: [QMF] Add compile-time deprecation warnings to the C++
QMF Agent/Console API headers.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15747/
-----------------------------------------------------------
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.
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
Re: Review Request 15747: [QMF] Add compile-time deprecation warnings to the
C++ QMF Agent/Console API headers.
Posted by Kenneth Giusti <kg...@apache.org>.
> 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
>
>
Re: Review Request 15747: [QMF] Add compile-time deprecation warnings to the
C++ QMF Agent/Console API headers.
Posted by Andrew Stitcher <as...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15747/#review29229
-----------------------------------------------------------
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)
- Andrew Stitcher
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
>
>
Re: Review Request 15747: [QMF] Add compile-time deprecation warnings to the
C++ QMF Agent/Console API headers.
Posted by Kenneth Giusti <kg...@apache.org>.
-----------------------------------------------------------
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 (updated)
-------
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