[wip] log manager POC #52
Conversation
lib/logging.dart
Outdated
|
|
||
| /// A shared manager instance whose recorded messages are logged to the | ||
| /// developer console. | ||
| final LogManager logManager = LogManager()..onRecord.listen((record) { |
There was a problem hiding this comment.
Maybe name this class by what it does.
DeveloperConsoleLogger, FilteredDeveloperConsoleLogger, etc?
| } | ||
|
|
||
| /// Returns a stream of messages added to loggers enabled by this manager. | ||
| Stream<LogRecord> get onRecord => _logController.stream; |
There was a problem hiding this comment.
Is this exposed for testing or for users? If you are implementing the full Logger api, might make sense to implement Logger.
| Stream<LogRecord> get onRecord => _logController.stream; | ||
|
|
||
| /// Enable (or disable) logging of messages sent to the given [logger]. | ||
| void enableLogging(String logger, {bool enable = true}) { |
There was a problem hiding this comment.
calling this variable logger here and elsewhere is a little counterintuitive to me as there is also a Logger class that means something different. Is there a good name for these logging channel names? Perhaps channelName if there isn't something snappier.
|
|
||
| // Fire events for new loggers. | ||
| _loggerAddedBroadcaster.stream.listen((String name) { | ||
| developer.postEvent('logging.logger.added', <String, dynamic>{ |
There was a problem hiding this comment.
logging.logger seems a little redundant.
maybe
ext.logging.added
or something similar?
| }) { | ||
| assert(name != null); | ||
| assert(callback != null); | ||
| final methodName = 'ext.dart.logging.$name'; |
There was a problem hiding this comment.
maybe just
ext.logging.name as all service extensions within a dart program are dart service extensions.
|
|
||
| /// Registers a service extension method with the given name and a callback to | ||
| /// be called when the extension method is called. | ||
| void _registerServiceExtension({ |
There was a problem hiding this comment.
probably make this method public but protected so you can override it for testing.
|
|
||
| /// Whether messages to the given [logger] should be logged by this manager. | ||
| /// @see [enableLogging] | ||
| bool shouldLog(String logger) => _enabledLoggers.contains(logger); |
|
@natebosch : this POC shows how our use cases could be addressed by adding on to current |
|
@kevmoo who has a lot of experience with this package. |
|
I'm not a huge fan of this change. Feel like something that should be in a
separate package. Happy to discuss more offline...
…On Thu, Nov 1, 2018 at 10:02 AM Jacob Richman ***@***.***> wrote:
@kevmoo <https://github.com/kevmoo> who has a lot of experience with this
package.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#52 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AABCis-eb0NFChTUrV6Ehl46d5r6Vbn7ks5uqykdgaJpZM4YFfrk>
.
|
|
Tabling for now. Thanks for all the productive conversation! |
Log Manager POC (see dart-lang/core#441).
Not for landing.
/cc @jacob314 @devoncarew @kevmoo @natebosch