Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Remove WriteVerbose statement from ConvertTo-Json#7487

Merged
daxian-dbw merged 2 commits into
PowerShell:masterPowerShell/PowerShell:masterfrom
devblackops:fix/converttojson-verbosedevblackops/PowerShell:fix/converttojson-verboseCopy head branch name to clipboard
Aug 10, 2018
Merged

Remove WriteVerbose statement from ConvertTo-Json#7487
daxian-dbw merged 2 commits into
PowerShell:masterPowerShell/PowerShell:masterfrom
devblackops:fix/converttojson-verbosedevblackops/PowerShell:fix/converttojson-verboseCopy head branch name to clipboard

Conversation

@devblackops

@devblackops devblackops commented Aug 9, 2018

Copy link
Copy Markdown
Contributor

PR Summary

Fix #7486.

In PR #6392, a WriteVerbose statement was added to ConvertTo-Json which I feel is unnecessary. On large or complex objects being serialized to json, this produces a ton of verbose messages which isn't adding any value to the end user. This PR removes that statement.

PR Checklist

@iSazonov

iSazonov commented Aug 9, 2018

Copy link
Copy Markdown
Collaborator

@devblackops I wonder do you see the verbose messages without -Verbose ?

@markekraus markekraus left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@devblackops Please also remove the message string which would no longer be used after this pr:

<data name="ConvertToJsonProcessValueVerboseMessage" xml:space="preserve">
<value>Processing object of type [{0}] at depth {1}</value>
</data>

@devblackops

Copy link
Copy Markdown
Contributor Author

@iSazonov The messages aren't shown without -Verbose but often will be inherited from cmdlets/functions that invoke ConvertFrom-Json that do have -Verbose specified.

It's easy enough to suppress the verbose output with -Verbose:False or set it via $PSDefaultParameterValues but in this case, these messages aren't really adding any value, hence the PR to remove them.

@devblackops

Copy link
Copy Markdown
Contributor Author

@markekraus I pushed a commit to remove the resource string.

@daxian-dbw daxian-dbw self-assigned this Aug 9, 2018

@dantraMSFT dantraMSFT left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I agree the verbose statement doesn't add much value.

It was added when StopProcessing was added so I expect it was done for debugging purposes.

@markekraus markekraus left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@daxian-dbw daxian-dbw merged commit 8374a5c into PowerShell:master Aug 10, 2018
@iSazonov

Copy link
Copy Markdown
Collaborator

@devblackops Thanks for your contribution!

xtqqczze added a commit to xtqqczze/PowerShell-PowerShell that referenced this pull request Jan 14, 2019
@xtqqczze xtqqczze mentioned this pull request Jan 14, 2019
11 tasks
@devblackops devblackops deleted the fix/converttojson-verbose branch January 15, 2019 21:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Remove unnecessary verbose message from ConvertTo-Json

5 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.