Skip to content

Remove static Logger API in favor of ILogger instance usage #488

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jun 3, 2017

Conversation

daviwil
Copy link
Contributor

@daviwil daviwil commented Jun 3, 2017

This is a pretty big change because it touches every usage of Logger.Write in the codebase and adds new parameters to pass around an ILogger instance to individual components. This is another incremental change toward the new PSES-as-a-framework model so that third-party extension modules will be able to add logs to the EditorServices.log file.

This change introduces the ILogger interface to abstract writing log
messages to an arbitrary sink so that this interface can be passed to
other components for logging behavior.  It also adds the FileLogger
implementation which is a refactored LogWriter implementation.
@daviwil daviwil added this to the 1.3.0 milestone Jun 3, 2017
This change continues the decoupling with the current codebase from the
static Logger classes and transitions to using ILogger instances for
writing log messages.  There are still a couple uses of a static ILogger
instance but they will be removed in an upcoming PR that refactors our
PSHost implementations.
@daviwil
Copy link
Contributor Author

daviwil commented Jun 3, 2017

Going to forge ahead on this because I've got some work to do tomorrow that depends on it! Let me know if you have any questions or concerns.

@daviwil daviwil merged commit 9b0f2a0 into PowerShell:master Jun 3, 2017
@daviwil daviwil deleted the add-ilogger branch June 3, 2017 22:33
@rkeithhill
Copy link
Contributor

I like this "DI" style approach. It is what I use in my code at work where I often have a NullLogger when I don't care about log output.

@daviwil
Copy link
Contributor Author

daviwil commented Jun 4, 2017

Yep, the refactoring that I'm doing is gradually moving everything to a DI model, you can see more of that in my latest PR 😄 I like being able to throw a dummy implementation in where something isn't needed. It'll definitely make some of the code easier to test.

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.

None yet

3 participants