Mir

Code review comment for lp:~vanvugt/mir/log-level

Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

I still think this is conflating (at least) two issues.

As things are (without this MP) specific loggers can be configured to handle different logging levels differently. E.g. --glog-minloglevel constrains the levels that are logged and --glog-stderrthreshold determines which levels are written to stderr.

Point 1: This functionality already exists in GlogLogger I see no reason to add it there again:
61 +void mir::examples::GlogLogger::enable(ml::Severity severity)

Reports can optionally write to the logger and, by specifying different logging levels for their messages, enable the above capabilities. But the reports should not decide how the logger handles these levels:

Point 2: The fact that a report is writing log messages is no guarantee that those messages should appear:

168 + logger->enable(ml::Severity::informational);

Users of the global logging functions can also exploit this capability by specifying different logging levels for their messages. What doesn't currently exist is an option (as already exists for the reports) of not writing these messages to the logger.

Trying to control the global logging functions via an enable(ml::Severity severity) function in the base class is primitive compared to what users can already do using glog.

We already have logging suppressed (by default - it's an option "--logging [on|off]") in the test framework and AFAICS providing the same (or similar) facility to clients would meet the desire for quiet clients without the complexity and inflexibility of this approach.

review: Needs Fixing

« Back to merge proposal