-
Notifications
You must be signed in to change notification settings - Fork 223
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
Remove Serilog, implement HostLogger integration, and deprecate net6.0 #2197
Conversation
be74dcd
to
9134c7d
Compare
Test for the debug adapter is stuck on reading the stream for some reason... PowerShellEditorServices/test/PowerShellEditorServices.Test.E2E/Processes/LoggingStream.cs Lines 42 to 43 in 3335b32
EDIT: StdioServerProcess used for testing has some loggerfactory references still. I'll investigate tomorrow. Should still be good for review, works otherwise for LSP and DAP as far as I can tell, just a faulty test. |
This is awesome! Serilog version mismatches have caused lots of issues for us when launching from our parent project. |
@andyleejordan this should be ready but it needs a package update to run tests. I can remove MEL.Debug if you want but I hope to expand it more throughout the extension. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Heck yeah.
test/PowerShellEditorServices.Test.E2E/DebugAdapterProtocolMessageTests.cs
Show resolved
Hide resolved
Also allows us to bump Microsoft.Extensions.Logging to 9.0.0.
828844a
to
1c07696
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thanks Justin!
Eliminate Serilog dependencies and wire up HostLogger to MEL directly after initialization.
I confirmed the HostLogger logs are showing up on the other side (this is using the new logoutputwindow on the vscode side but should look fine with the old LSP trace as well)
This shows where the file based logging stops and then resumes inside LSP communication
As part of the resolutions, this also drops net6.0 support due to some library changes (which was going to happen anyways as net6.0 is EOL as of November)
Fixes PowerShell/vscode-powershell#5084