----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23977/#review48844 -----------------------------------------------------------
Ship it! Looks like a worthwhile cleanup to me! /trunk/qpid/cpp/src/qpid/broker/amqp/ManagedConnection.cpp <https://reviews.apache.org/r/23977/#comment85582> Should probably delete the preceding comment as well (which looks to be already stake), just in case anyone reading decides to re-enable this :-) /trunk/qpid/cpp/src/qpid/broker/management-schema.xml <https://reviews.apache.org/r/23977/#comment85583> Not that it really matters, but 'unused' would perhaps be more accurate than deprecated here. It would be quite a useful stat, but 0-10 doesn't make it easy to track (1.0 by contrast would). /trunk/qpid/cpp/src/qpid/broker/management-schema.xml <https://reviews.apache.org/r/23977/#comment85584> Is it not the case that 'federation link' and 'incoming' indicate different things? Though all outgoing federation links are clearly incoming=false, is there not value in identifying incoming federation links as such? Of course its probably only possible to determine that from the connection properties, which are already included in the schema, so perhaps nothing further is needed. And if the field is never actually set at present anyway then the point is somewhat moot since all you are doing is updating the description. - Gordon Sim On July 28, 2014, 11:23 a.m., Pavel Moravec wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/23977/ > ----------------------------------------------------------- > > (Updated July 28, 2014, 11:23 a.m.) > > > Review request for qpid, Gordon Sim, Kim van der Riet, and Ted Ross. > > > Bugs: QPID-5929 > https://issues.apache.org/jira/browse/QPID-5929 > > > Repository: qpid > > > Description > ------- > > Apart from marking deprecated QMF fields with description "Deprecated" in > management-schema.xml (just to document this fact anywhere), I suggest > simplifying the code by removing some bits relevant to the deprecated fields. > In particular: > > - remove some Ftd related methods (dead code) > - don't call session's set_detachedLifespan(0) as that is already the default > - don't call connection's set_shadow(false) as that is already the default > - userProxyAuth is in fact deprecated; the only place where it can have value > is in cpp/src/qpid/broker/amqp/ManagedConnection.cpp, line: > > connection = _qmf::Connection::shared_ptr(new _qmf::Connection(agent, this, > parent, id, !brokerInitiated, brokerInitiated, "AMQP 1.0")); > > (the last but not least parameter). But I see it use-less to have > userProxyAuth = !incoming property (or does it have some sense? then > amqp_0_10 should set it as well, now it sets false everytime). > > In either case, userProxyAuth property from nonQMF connection class can be > removed - it's everytime set to false. Propagating its value, > isUserProxyAuth() equals to isFederationLink() method and can be replaced. > > > Diffs > ----- > > /trunk/qpid/cpp/src/qpid/broker/Queue.h 1613795 > /trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1613795 > /trunk/qpid/cpp/src/qpid/broker/SemanticState.cpp 1613795 > /trunk/qpid/cpp/src/qpid/broker/SessionState.cpp 1613795 > /trunk/qpid/cpp/src/qpid/broker/amqp/ManagedConnection.cpp 1613795 > /trunk/qpid/cpp/src/qpid/broker/amqp/ManagedSession.cpp 1613795 > /trunk/qpid/cpp/src/qpid/broker/amqp_0_10/Connection.h 1613795 > /trunk/qpid/cpp/src/qpid/broker/amqp_0_10/Connection.cpp 1613795 > /trunk/qpid/cpp/src/qpid/broker/management-schema.xml 1613795 > /trunk/qpid/cpp/src/qpid/legacystore/management-schema.xml 1613795 > /trunk/qpid/cpp/src/qpid/linearstore/management-schema.xml 1613795 > > Diff: https://reviews.apache.org/r/23977/diff/ > > > Testing > ------- > > automated tests passed > > > Thanks, > > Pavel Moravec > >
