Drop hardcoded list of known logger names #2194
Conversation
Now that there is a split with some of these logger names coming from `build_runner_core` it's not as easy to roll out changes which introduce new logger names. The only cases where we do want to show the logger name are the ones where we are running actions and will always fit the pattern of `'$builderLabel on $input'`.
| String _recordHeader(LogRecord record, bool verbose) { | ||
| var maybeSplit = record.level >= Level.WARNING ? '\n' : ''; | ||
| return verbose || !knownNames.contains(record.loggerName) | ||
| return verbose || record.loggerName.contains(' on ') |
There was a problem hiding this comment.
Not a big fan of the pattern matching here... if we instead removed all the custom loggers and used a single common logger we could check for that one specifically and it would be a fair bit better I think?
There was a problem hiding this comment.
Oh although then we wouldn't get the names in verbose mode...
There was a problem hiding this comment.
Another option could be creating a custom LogRecord type? That would be safer but also has other disadvantages, although that class hasn't changed in ages so it is unlikely we would run into issues.
There was a problem hiding this comment.
I'd be in favor of dropping the names in verbose mode and simplifying down to a single common logger, but they can be handy sometimes so I can see an argument for keeping them.
I'm not excited by the idea of a custom LogRecord implementation, though I think all the cases for logs that we do want to print the name go through BuildForInputLogger which might give us some leverage to handle it...
There was a problem hiding this comment.
dart-lang/webdev#305 added a dependency on the only logger names we want printed containing a space. I'll use the same criteria here.
In the long term we'll likely want to prefix all the non-shown logger names with build.
There was a problem hiding this comment.
In the long term we'll likely want to prefix all the non-shown logger names with
build.
Although now that I think about it this would break for webdev haha
This assumes the logger names do not contain spaces. Based off of dart-lang/build#2194 this assumption appears correct. Closes #301
Now that there is a split with some of these logger names coming from
build_runner_coreit's not as easy to roll out changes which introducenew logger names. The only cases where we do want to show the logger
name are the ones where we are running actions and will always fit the
pattern of
'$builderLabel on $input'.