-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5616/#review9134
-----------------------------------------------------------

Ship it!


I don't particularly like the term 'Model' in this context. Especially for the 
generated code, 'Management' seems much more appropriate (it won't be logged 
unless management is enabled and it is logging the stats the management agent 
records). For the events I would also prefer using Management or Broker. I 
assume the desire is to have a unique category that only selects these entity 
lifecycle related statements. Perhaps 'Lifecycle' would be an alternative?

That said, this is subjective and not the most important aspect. Your patience 
and persistence in accomodating my incessant whining has been much appreciated! 
I wouldn't object to inclusion as is if my points above do not convince you. 
This is a nice, clean addition. Good job! 

- Gordon Sim


On July 13, 2012, 1:47 a.m., Chug Rolke wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5616/
> -----------------------------------------------------------
> 
> (Updated July 13, 2012, 1:47 a.m.)
> 
> 
> Review request for qpid, Alan Conway, Gordon Sim, and Ted Ross.
> 
> 
> Description
> -------
> 
> This patch adds a new log category [Configuration] and publishes a bunch of 
> information at info level in the format:
> [Configuration] <object> <event>, where <object> is one of Connection, 
> Session, Queue, Subscription, Exchange or Binding, and <event> is one of 
> created or closed with connection also getting a setUser.
> The bulk of the patch involves passing the necessary strings down to the 
> object creators so that they can emit the log.
> 
> 
> This addresses bug QPID-4079.
>     https://issues.apache.org/jira/browse/QPID-4079
> 
> 
> Diffs
> -----
> 
>   trunk/qpid/cpp/include/qpid/log/Statement.h 1360512 
>   trunk/qpid/cpp/managementgen/qmf-gen 1360512 
>   trunk/qpid/cpp/managementgen/qmfgen/generate.py 1360512 
>   trunk/qpid/cpp/managementgen/qmfgen/templates/Class.cpp 1360512 
>   trunk/qpid/cpp/src/CMakeLists.txt 1360512 
>   trunk/qpid/cpp/src/Makefile.am 1360512 
>   trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1360512 
>   trunk/qpid/cpp/src/qpid/broker/Connection.cpp 1360512 
>   trunk/qpid/cpp/src/qpid/broker/ConnectionHandler.cpp 1360512 
>   trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1360512 
>   trunk/qpid/cpp/src/qpid/broker/SessionAdapter.cpp 1360512 
>   trunk/qpid/cpp/src/qpid/log/Statement.cpp 1360512 
> 
> Diff: https://reviews.apache.org/r/5616/diff/
> 
> 
> Testing
> -------
> 
> Passes cmake test and automake make check.
> 
> 
> Thanks,
> 
> Chug Rolke
> 
>

Reply via email to