Open nickgw opened 1 year ago
All modified and coverable lines are covered by tests :white_check_mark:
Comparison is base (
196d491
) 90% compared to head (79d051c
) 86%. Report is 1 commits behind head on main.:exclamation: Current head 79d051c differs from pull request most recent head a87d39d. Consider uploading reports for the commit a87d39d to get more accurate results
The other PR is merged, you can rebase this against main branch now.
@nickgw let me know when this one is ready for review.
@johlju , I think this is ready for a review when you get a chance. I've written unit tests for everything except GetCurrentStatus()
, Modify()
, Set()
and Get()
at this point.
I think my two outstanding questions are:
Is shimming setting LatestVersion
and VersionRequirement
readonly properties into AssertProperties()
the right move? AssertProperties()
is the only method in PSResource
that always gets called which is why I put it there.
How should I handle AllowPrerelease
? I kind of want to gate it to only allow RequiredVersion
, Latest
or no versionrequirement at all. Trying to massage pre-releases in with MinimumVersion
& MaximumVersion
kind of makes my head hurt but I haven't thought about it too much.
As an fun exercise I created a class that might remove some of the logic from the DSC resource and could be re-used in other ways (in the future). I just did a PoC of my thoughts. Let me if you think it could help anything. Please re-use any part you like, if it helps.
See the code in the gist here: https://gist.github.com/johlju/5f1ed2fe2f510c9e4272dd942a389723
classDiagram
class PSResourceObject {
+string Name
+version Version
+string PreRelease
+PSResourceObject()
+PSResourceObject(string Name)
+PSResourceObject(string Name, version Version)
+PSResourceObject(string Name, version Version, string PreRelease)
+CompareTo(object) int32
+Equals(object) boolean
+GetInstalledResource(string)$ PSResourceObject[]
+GetMinimumInstalledVersion(string)$ PSResourceObject
+GetMinimumInstalledVersion(PSResourceObject[])$ PSResourceObject
+GetMaximumInstalledVersion(string)$ PSResourceObject
+GetMaximumInstalledVersion(PSResourceObject[])$ PSResourceObject
+IsPreRelease() Boolean
}
class IComparable {
<<Interface>>
CompareTo(object) int32
}
class IEquatable {
<<Interface>>
Equals(object) boolean
}
IComparable <|-- PSResourceObject : implements
IEquatable <|-- PSResourceObject : implements
I gonna be away tomorrow, but get back to a review of this on friday. π
How should I handle AllowPrerelease? I kind of want to gate it to only allow RequiredVersion, Latest or no versionrequirement at all. Trying to massage pre-releases in with MinimumVersion & MaximumVersion kind of makes my head hurt but I haven't thought about it too much.
Would the class I put together help with this? It could compare two objects. We could something like below (assuming the class is renamed to PSResourceObject
in lack of a better name).
Assuming the current state contain version 1.0.0 and the minimum required version is 1.0.1-preview1.
$currentStateResource = [PSResourceObject] @{
Name = 'MyModule'
Version = '1.0.0'
}
$minimumRequiredResource = [PSResourceObject] @{
Name = 'MyModule'
Version = '1.0.1'
PreRelease = 'preview1'
}
$minimumRequiredResource.CompareTo($currentStateResource)
That would result in 1
meaning the minumim version required is not installed.
1
= minimum version need to be installed0
= minimum version is compliant, the exact minimum version is installed-1
= minimum version is compliant, there is already a higher version installed than the minimum versionAdded example output to the gist's README.md (link above).
I will continue the review once these are done (or when I have time). I only got half-way through Modify() (and connected methods). π
This is an old PR and one that we actually shouldn't take Reason being that PowerShellGet v3.0.0-beta9 included a resource for it along with one for PSRepositories, which means that we should look in future to deprecate the resource in this repository that does this We also should we asking for the resources in future that comes from the renamed PSResourceGet Module instead as per https://github.com/PowerShell/PSResourceGet/issues/1697
We said before the start of this PR that we add it to this repository and if in the future any officially supported version will get to full release, then we can codiser removing them from here, or rename them. This is because there were DSC resource in PowerShellGet but they were moved out in a separate repo and never re-relelased. Then PSResourceGet was invented and that did not have any resources either, and looking at the pace PSResourceGet is moving I don't think DSC resources are on the radar for a long time... or never. π
So I suggest we should accept this PR.
When (if) Windows PowerShell is released with PSResourceGet we could move the resources (once merge,in another future PR) to use Install-PSResourceGet
, or have a switch to be able to .
Pull Request (PR) description
New resource to manage PowerShell Resources
PSResource
This Pull Request (PR) fixes the following issues
Task list
This change isβ