ConvertFrom-Json: Unwrap Collections by default #10861
Conversation
|
|
|
/cc @mklement0 Could you please review? |
|
Looked at the failing checks. I'm not sure what mandatory parameter I'm supposed to be passing to I can change the documentation for the new parameter, I pretty much just copied it from the Write-Output cmdlet. Starting the sentence with "Get or sets" strikes me as awkward - any proposals for the text? |
src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/ConvertFromJsonCommand.cs
Outdated
Show resolved
Hide resolved
test/powershell/Modules/Microsoft.PowerShell.Utility/ConvertFrom-Json.Tests.ps1
Outdated
Show resolved
Hide resolved
|
Everything now looks good to me - I'm also baffled by the two Codacy failures:
@SteveL-MSFT, since you seem to have set up Codacy, any ideas? |
Adds documentation for new parameter added with PowerShell/PowerShell#10861
|
Codacy is probably using existing stable versions of PS as reference. Since we're adding a parameter here, we can expect it to complain. |
|
Thanks, @vexx32. It's curious that instead of complaining about an unrecognized parameter it admonishes checking for mandatory ones - but perhaps it's a catch-all message for any unrecognized syntax (can this be customized?). I haven't participated much in code reviews, so I'm asking innocently:
|
|
This particular warning seems to be a new thing, I think? I haven't seen it much until now, at least. Codacy issues have always been considered fairly low-priority and not blocking, though -- we have Codacy completely ignoring C# at the moment as its linter isn't entirely up to speed and the newer C# features confuse it a lot. Its PS linting is fairly good, but it's not mandatory to fix all issues -- some of them, like this one, we simply can't, after all! :slight_smile: It's generally up to the folks reviewing whether we ask authors to fix Codacy issues, from what I've seen. 🙂 |
src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/ConvertFromJsonCommand.cs
Outdated
Show resolved
Hide resolved
Changes the default behavior of ConvertFrom-Json to follow the usual
collection-unwrapping behavior.
With this change `('[1,2]' | ConvertFrom-Json | Measure-Object).Count`
returns 2, instead of 1.
Provides a NoEnumerate switch similar to the Write-Output command to
revert to the old behavior.
See issue PowerShell#3424.
|
|
@danstur Thanks for your contribution. |
Adds documentation for new parameter added with PowerShell/PowerShell#10861
|
🎉 Handy links: |
PR Summary
Changes the default behavior of ConvertFrom-Json to follow the usual collection-unwrapping behavior. With this change
('[1,2]' | ConvertFrom-Json | Measure-Object).Countreturns 2, instead of 1.Provides a NoEnumerate switch similar to the Write-Output command to revert to the old behavior.
See issue #3424.
PR Context
Improves the user experience for the majority of people by changing ConvertFrom-Json's behavior to match the default behavior when dealing with collections. See for example this or this stackoverflow question for examples of people falling into this trap.
PR Checklist
.h,.cpp,.cs,.ps1and.psm1files have the correct copyright header