Repository navigation
Port five workloads to the native PowerShell workload engine - #176
AmirMS (AmelBawa-msft) wants to merge 1 commit into
Conversation
ranm-msft
left a comment
ranm-msft
left a comment
There was a problem hiding this comment.
One substantive note inline on powershell-analyzer.ps1 about the settings.json round trip. It is inherited from the workload this replaces, so I am treating it as a scope call rather than a blocker.
The rest of the conversion reads cleanly: the workload definitions are straightforward phase manifests, the new manual_test: true entries carry an explicit reason, and the AnyVersion catalog entry is commented.
| $settings = Read-DevConfigVSCodeSettings | ||
| $settings['powershell.scriptAnalysis.enable'] = $true | ||
| $settings['powershell.scriptAnalysis.settingsPath'] = $analyzerSettingsPath | ||
| ConvertTo-Json -InputObject $settings -Depth 100 | |
There was a problem hiding this comment.
Read-DevConfigVSCodeSettings turns the user's JSONC settings.json into a hashtable (comments stripped at L55-56 so ConvertFrom-Json will accept it), and this line serializes that hashtable straight back over the original file. Comments are not represented in the hashtable, so configuring the analyzer silently deletes every comment from the developer's VS Code settings and reflows the file. settings.json is JSONC and people do comment it.
To be fair this is carried forward rather than introduced here - the deleted src/Workloads/powershell/configuration.winget did the same Read-VSCodeSettings strip plus ConvertTo-Json -Depth 100 | Set-Content. So this is a scope call, not a regression.
Two related notes:
- The block-comment regex at L55 is not string-aware, so a value like
"example": "keep/*this*/text"parses askeeptextand gets written back corrupted. - Dropping L55-56 would not fix it on its own: PowerShell 7's
ConvertFrom-Jsonalready tolerates comments, and they would still be lost on theConvertTo-Jsonround trip. The write path has to preserve the original source text.
Would you consider a source-preserving edit that touches only the two powershell.scriptAnalysis.* properties? src/tests/calm-os/powershell-analyzer-checks.ps1 already seeds a fixture containing a comment but only asserts the unrelated files.autoSave key survives, so it passes with the comment deleted - a raw-content assertion there would pin the behaviour either way. If that is out of scope for a conversion PR, tracking it as a follow-up is fine by me.
2 participants
Summary
Validation
sqlcmd.git diff --checkpassed.