Skip to content

Comments

Enable Write-Information to accept $null#8774

Merged
anmenaga merged 4 commits intoPowerShell:masterfrom
SteveL-MSFT:null-write-information
Jan 31, 2019
Merged

Enable Write-Information to accept $null#8774
anmenaga merged 4 commits intoPowerShell:masterfrom
SteveL-MSFT:null-write-information

Conversation

@SteveL-MSFT
Copy link
Member

@SteveL-MSFT SteveL-MSFT commented Jan 29, 2019

PR Summary

Write-Information currently errors if $null is passed.

PR Context

There's no benefit to having it error out if $null is passed. If a pipeline is sending objects to Write-Information, it would make sense for the cmdlet to handle $null which in this case is a no-op.

PR Checklist

@SteveL-MSFT
Copy link
Member Author

The codefactor issue is in existing code that wasn't changed and also complaining about Hungarian notation which it isn't.

}

It "Write-Information accespts `$Null" {
$null | Write-Information | Should -BeNullOrEmpty
Copy link
Collaborator

Choose a reason for hiding this comment

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

Rather than "NullOrEmpty", we should probably be specific about what this case does, and test it by checking the information stream for the result if possible.

Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure what you're proposing to check here. Previously it was returning a non-terminating error. I can change it to stop on error which would cause the test to fail without this change. The OrEmpty of the assertion only applies if the input is a string anyways.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Write-Information doesn't write to pipeline so the test is misleading. It is more clear to check Information stream contains $null or check -NotThrow.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, I see what you guys are saying now. Will make a change.

@SteveL-MSFT
Copy link
Member Author

The codefactor issue is wrong

@anmenaga
Copy link

@rjmholt @iSazonov Plase review this PR again when you get a chance. Thank you.

@anmenaga anmenaga merged commit 313c8c0 into PowerShell:master Jan 31, 2019
@iSazonov iSazonov added the CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log label Feb 1, 2019
@SteveL-MSFT SteveL-MSFT deleted the null-write-information branch June 6, 2020 02:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants