Skip to content

Conversation

@devoncarew
Copy link
Contributor

@grouma

setLogHandler((level, message, {verbose}) {
daemonDomain
.sendEvent('daemon.log', {'level': '$level', 'message': message});
if (verbose == true) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems a bit odd to be based on the --verbose flag, what is the context?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure this is what we want to do?

I feel like these messages should always be displayed as they provide build action updates, e.g.:

[INFO] Generating build script completed, took 375ms
[INFO] Setting up file watchers completed, took 13ms
[INFO] Waiting for all file watchers to be ready completed, took 116ms
[INFO] Reading cached asset graph completed, took 333ms
[INFO] Checking for updates since last build completed, took 831ms
[INFO] Running build completed, took 331ms
[INFO] Caching finalized dependency graph completed, took 195ms
[INFO] Succeeded after 534ms with 0 outputs (0 actions)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it definitely depends on the types of messages that it's sending w/ verbose==true. If these should be user facing in general, then we'd want to send them all to daemon.log.

One way to think about it is, would these messages show as part of the output for a normal flutter run from the cli? Or, would they only show w/ a flutter run -v run?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated this to send all messages from here through 'daemon.log'.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah these should all be displayed. Sorry I forgot to change the structure of the message. Thanks for the PR.

@devoncarew devoncarew merged commit 385f6ea into master Apr 19, 2019
@grouma grouma deleted the daemon_log branch May 28, 2019 22:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants