Microsoft.WinGet.DSC improvements (and breaking change) #4795
Conversation
…ome work done on both the resource class and docs for sources
…inor manual testing done for package resource
| context.Reporter.Error() << Resource::String::WinGetResourceUnitMissingRequiredArg(Utility::LocIndView{ UnitType_WinGetSource }, "Name"_liv) << std::endl; | ||
| foundIssues = true; | ||
| } | ||
| if (source.Arg.empty()) |
There was a problem hiding this comment.
Will this require arg even when ensuring the source is absent?
I'm just thinking about how that may be inconsistent with winget source remove <name>. Although, for DSC, requiring the arg may be a good thing in case a user wants to remove a source and replace it with a new source of the same name but with a different arg. . .
There was a problem hiding this comment.
This code wasn't originally written to be Ensure aware. You are correct that during Absent only the Name is required.
| { | ||
| $removeSource = $true | ||
| } | ||
| else |
There was a problem hiding this comment.
I think I like more the else comment like 265, but at least both should be consistent
| # Sets the desired properties. | ||
| [void] Set() | ||
|
|
||
| [bool] hidden TestAgainstCurrent([WinGetSource]$currentSource) |
There was a problem hiding this comment.
I had to go back on why there's no comparison of Name. You don't want to call Get here because you don't want two Get-WinGetSource in Set. Maybe add a comment?
| [DscProperty(Mandatory)] | ||
| [Hashtable[]]$Sources | ||
|
|
||
| [string]$Argument |
There was a problem hiding this comment.
do we need [ValidateNotNullOrWhiteSpace()] here?
There was a problem hiding this comment.
No, because if Ensure = Absent we don't want to require an argument value (although from my testing it seems like the validation only happens during assignment). If someone provides an empty argument they will still get an error, just from deeper in the stack.
| @@ -52,7 +52,7 @@ else | |||
| { | |||
| Write-Host "PowerToys 0.65.0 is not installed." | |||
|
|
|||
| Invoke-DscResource @resource -Method Set | |||
| #Invoke-DscResource @resource -Method Set | |||
There was a problem hiding this comment.
You don't want the sample to install PowerToys?
There was a problem hiding this comment.
I forgot to revert that after testing the script.
The goal of this PR is to get these resources to a state where we feel ready to release the module without the prerelease tag. The
WinGetUserSettings,WinGetAdminSettings, andWinGetPackageManagerresources were deemed acceptable in their current state. TheWinGetSourcesresource was decided to be overly complicated and likely not heavily used yet (and containing some bugs), so it was removed in favor of a newWinGetSourceresource for managing individual sources. TheWinGetPackageresource was updated to be better in line with best practices but should still be compatible unless one was usingGet.Change
The
WinGetSourcesresource was replaced with aWinGetSourceresource that manages a single source. TheWinGetSourcesresource can continue to be used by specifying one of the module versions that contains it, but once released, new configurations should prefer theWinGetSourceresource from the latest module.While there is no way to include or discover help via the module, I documented the resources similarly to the cmdlet documentation format. These can be used as the basis for official documentation, or we can put a link to them in the PowerShell Gallery listing (maybe updating the
Project Siteto point to a new landing page for the DSC resources).The updates to
WinGetPackageaim to have it follow best practices and make general improvements:Sourceas aKey$thisin calls toGet/Set/TestGetIsInstalledis reported viaEnsure(true =>Present, false =>Absent)InstalledVersionis reported viaVersionIsUpdateAvailableis reported viaUseLatest(they will have the opposite boolean value from each other)Any existing (non-
Get) uses of the resource should continue to work as they did before.Smaller updates worth mentioning:
winget configure validatefunctionality to handle the newWinGetSourceresource instead ofWinGetSources.VersionandUseLatestto theWinGetPackageresource.UseLatestwill override anyVersionprovided if it is true.PackageCatalogInfo::TrustLevelproperty reportingNoneimproperly if the internal trust level contained bothTrustedand another flag.Validation
Manual verification of basics locally to ensure golden path functionality.
Updated existing tests to cover changes, including adding new fields.
Microsoft Reviewers: Open in CodeFlow