Skip to content

Use correct isError parameter with Write-Log when logging an error - #11592

Closed
James Truher (JamesWTruher) wants to merge 278 commits into
PowerShell:masterfrom
JamesWTruher:WriteLogFix001
Closed

Use correct isError parameter with Write-Log when logging an error#11592
James Truher (JamesWTruher) wants to merge 278 commits into
PowerShell:masterfrom
JamesWTruher:WriteLogFix001

Conversation

@JamesWTruher

Copy link
Copy Markdown
Collaborator

PR Summary

It looks like the wrong parameter was being used with Write-Log causing difficulties when logging errors.

PR Context

PR Checklist

Comment thread build.psm1 Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

James Truher (@JamesWTruher) Could you please elaborate a bit why -message needs to be specified?
It is a positional parameter (position = 0), so it looks to me should have worked as expected.

PS:4> gcm Write-Log | % Definition

    param
    (
        [Parameter(Position=0, Mandatory)]
        [ValidateNotNullOrEmpty()]
        [string] $message,

        [switch] $isError
    )
    if ($isError)
    {
        Write-Host -Foreground Red $message
    }
    else
    {
        Write-Host -Foreground Green $message
    }
    #reset colors for older package to at return to default after error message on a compilation error
    [console]::ResetColor()

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Generally, positional parameters are less preferred than being explicit (we have an analyzer rule for this). While I was making the changes elsewhere to the use of Write-Log, I thought I should do it here.

@TravisEz13

Copy link
Copy Markdown
Member

James Truher (@JamesWTruher) Please resolve conflicts

Comment thread build.psm1
@@ -314,7 +314,7 @@ function Start-PSBuild {
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

James Truher (@JamesWTruher), your last commit had 1 failures in PowerShell-CI-windows
Remove-Item UnAuthorized Access.Access-denied test for removing a folder

Expected exactly 'RemoveItemUnauthorizedAccessError,Microsoft.PowerShell.Commands.RemoveItemCommand', but got $null.
at <ScriptBlock>, D:\a\1\s\test\powershell\Modules\Microsoft.PowerShell.Management\FileSystem.Tests.ps1: line 1424
1424:         Get-Content $errorFile | Should -BeExactly 'RemoveItemUnauthorizedAccessError,Microsoft.PowerShell.Commands.RemoveItemCommand'

* Autofix `RCS1036: Remove redundant empty line`

* Autofix `RCS1037: Remove trailing white-space`

* Autofix `RCS1033: Remove redundant boolean literal`

* Autofix: `RCS1038: Remove empty statement`

* Autofix `RCS1041: Remove empty initializer`

* Autofix `RCS1097: Remove redundant 'ToString' call

* Autofix: `RCS1420: Operator is unnecessary`
@ghost ghost added the Stale label Jun 11, 2020
@ghost

Copy link
Copy Markdown

This pull request has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 15 days. It will be closed if no further activity occurs within 10 days of this comment.

@ghost ghost removed the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label Jun 18, 2020
@TravisEz13

Travis Plunk (TravisEz13) commented Jun 18, 2020

Copy link
Copy Markdown
Member

Merged #12989

@TravisEz13

Copy link
Copy Markdown
Member

This was not rebased correctly.

@JamesWTruher
James Truher (JamesWTruher) deleted the WriteLogFix001 branch June 18, 2020 19:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

WG-Quality-Test issues in a test or in test infrastructure

Projects

None yet

Development

Successfully merging this pull request may close these issues.