Skip to content

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

Closed
JamesWTruher wants to merge 278 commits intoPowerShell:masterfrom
JamesWTruher:WriteLogFix001
Closed

Use correct isError parameter with Write-Log when logging an error#11592
JamesWTruher wants to merge 278 commits intoPowerShell: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

@ghost ghost assigned daxian-dbw Jan 15, 2020
@JamesWTruher JamesWTruher added the WG-Quality-Test issues in a test or in test infrastructure label Jan 15, 2020
@daxian-dbw daxian-dbw removed their assignment Jan 24, 2020
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.

@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 TravisEz13 requested a review from daxian-dbw February 3, 2020 20:58
@TravisEz13
Copy link
Copy Markdown
Member

@JamesWTruher Please resolve conflicts

@@ -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.

@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'

adityapatwardhan and others added 17 commits February 19, 2020 12:45
adityapatwardhan and others added 4 commits June 9, 2020 10:30
* 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

ghost commented Jun 11, 2020

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 Stale label Jun 18, 2020
@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
Copy link
Copy Markdown
Member

TravisEz13 commented Jun 18, 2020

Merged #12989

@TravisEz13 TravisEz13 closed this Jun 18, 2020
@TravisEz13
Copy link
Copy Markdown
Member

This was not rebased correctly.

@JamesWTruher 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.