Skip to content

Conversation

@TheRealPiotrP
Copy link

Enables subscribing a logging callback that will be invoked by all instances of Logger.

@DustinCampbell
Copy link
Member

CI failed with the same weird array of nulls we saw in my PR. Maybe that was unrelated to issue I needed to fix? I'm wondering if the problem is related to the fact that VS Code 1.18 just shipped, so the stable build used for test was updated.

Copy link
Member

@DustinCampbell DustinCampbell left a comment

Choose a reason for hiding this comment

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

Nice change. Just need to get that pesky CI fixed.

* Copyright (c) Microsoft Corporation. All rights reserved.
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/
let Subscriber: (message: string) => void;
Copy link
Member

Choose a reason for hiding this comment

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

minor, minor nit: blank line before

@TheRealPiotrP TheRealPiotrP force-pushed the dev/piotrp/logOutput branch 3 times, most recently from 31b0fa5 to 1c83be3 Compare November 17, 2017 08:20
@TheRealPiotrP
Copy link
Author

@DustinCampbell @rchande @akshita31 this change ended up being super noisy in the logs. However, I was able to make the UX quite nice anyways :)

The latest revision of this change:

  • writes logs to log files in a .logs directory
  • after the test script runs it cat's the log files to the Travis build log
  • It employs a loosely documented Travis trick to enable log folding around the printed log files

Check it out!
screen shot 2017-11-17 at 12 28 32 am

@TheRealPiotrP TheRealPiotrP merged commit 43c9aa1 into dotnet:master Nov 17, 2017
@TheRealPiotrP TheRealPiotrP deleted the dev/piotrp/logOutput branch November 17, 2017 08:42
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.

3 participants