----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/10799/#review19785 -----------------------------------------------------------
Ship it! Nice - something I had in mind way back when and never got around to it. My comments are all style issues, the code looks good. Need to update the user doc to explain how to use this, suggest updating the broker book at qpid/doc/book/src/cpp-broker trunk/qpid/cpp/include/qpid/log/Options.h <https://reviews.apache.org/r/10799/#comment40807> Maybe unselectors rather than disselectors? trunk/qpid/cpp/include/qpid/log/Selector.h <https://reviews.apache.org/r/10799/#comment40808> prefer struct over classwhen its all public. trunk/qpid/cpp/include/qpid/log/Selector.h <https://reviews.apache.org/r/10799/#comment40810> Type name should be capitalized. LFN isn't a well-known abbreviation, could the name be more descriptive? trunk/qpid/cpp/include/qpid/log/Selector.h <https://reviews.apache.org/r/10799/#comment40809> perfer not abbreviating enable/disable - Alan Conway On April 26, 2013, 5:13 p.m., Chug Rolke wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/10799/ > ----------------------------------------------------------- > > (Updated April 26, 2013, 5:13 p.m.) > > > Review request for qpid and Alan Conway. > > > Description > ------- > > This patch adds a --log-disable option. > > Internally the enabled and disabled options are stored in parallel tables so > that enable and disable options may be specified and processed in any order > yet return the same enable/disable decision. > > The management methods getLogLevel and setLogLevel express disabled options > with a leading '!'. For example in getLogLevel option "--log-enable debug+" > becomes "debug+" and option "--log-disable debug+" becomes "!debug+". > > > This addresses bug QPID-4651. > https://issues.apache.org/jira/browse/QPID-4651 > > > Diffs > ----- > > trunk/qpid/cpp/include/qpid/log/Options.h 1476242 > trunk/qpid/cpp/include/qpid/log/Selector.h 1476242 > trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1476242 > trunk/qpid/cpp/src/qpid/log/Logger.cpp 1476242 > trunk/qpid/cpp/src/qpid/log/Options.cpp 1476242 > trunk/qpid/cpp/src/qpid/log/Selector.cpp 1476242 > trunk/qpid/cpp/src/tests/BrokerFixture.h 1476242 > trunk/qpid/cpp/src/tests/dynamic_log_level_test 1476242 > trunk/qpid/cpp/src/tests/logging.cpp 1476242 > > Diff: https://reviews.apache.org/r/10799/diff/ > > > Testing > ------- > > New self test is added to exercise SelectorElememt to prove it's parsing > results. > Tests added to check Option processing and Selector setting results. > Dynamic test added to check --log-disable setting and to check management > setting of disabled elements. > > > Thanks, > > Chug Rolke > >
