Remove redundant check from Start-EditorServices#2333
Open
ChrisTX wants to merge 1 commit into
Open
Conversation
6f76c6e introduced a correct change to the ValidateSet, but added a redundant check that is already handled in `StartEditorServicesCommand.cs` in its `StartLogging()` routine. In fact, exchanging the values in Start-EditorServices will suppress the warning about the old PSES log levels that `StartLogging()` would normally emit. However, the check is actually harmful on top of this issue since it doesn't check if LogLevel is even set. If LogLevel is `$null`, then the default option ``` default { $LogLevel } ``` isn't valid PowerShell anymore: ``` The variable cannot be validated because the value $null is not a valid value for the LogLevel variable. ```
Contributor
There was a problem hiding this comment.
Pull request overview
This PR removes a redundant log-level translation block from Start-EditorServices.ps1. The translation of legacy PSES log levels (Diagnostic, Verbose, Normal) to Microsoft.Extensions.Logging (MEL) levels is already handled in the StartLogging() routine of StartEditorServicesCommand.cs. The script-side block was not only redundant but harmful: it suppressed the deprecation warning emitted by the C# code, and its default { $LogLevel } branch would throw a ValidateSet error when $LogLevel was $null.
Changes:
- Removed the legacy-to-MEL log level translation
switchblock from the script. - Normalized indentation (tabs → spaces) on the
$SessionDetailsPathparameter.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
6f76c6e introduced a correct change to the ValidateSet, but added a redundant check that is already handled in
StartEditorServicesCommand.csin itsStartLogging()routine. In fact, exchanging the values in Start-EditorServices will suppress the warning about the old PSES log levels thatStartLogging()would normally emit.However, the check is actually harmful on top of this issue since it doesn't check if LogLevel is even set.
If LogLevel is
$null, then the default optionisn't valid PowerShell anymore: