Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merge xWindowsRestore into ComputerManagementDsc #364

Open
cohdjn opened this issue Mar 14, 2021 · 11 comments · May be fixed by #373
Open

Merge xWindowsRestore into ComputerManagementDsc #364

cohdjn opened this issue Mar 14, 2021 · 11 comments · May be fixed by #373
Labels
discussion The issue is a discussion. resource proposal The issue is proposing a new resource in the resource module.

Comments

@cohdjn
Copy link
Contributor

cohdjn commented Mar 14, 2021

Description

I want to configure System Restores for new Windows 10 machines and this module doesn't have those resources available, but I did find an old PowerShell module (xWindowsRestore) that @gaelcolas and the PowerShell Team wrote back in 2014. Rather than having a one-off module just handling system restores, I'd like to merge that module into ComputerManagementDsc with all the Pester testing and other dsccommunity goodies.

The other interesting note is that when you click the Project Site link in the gallery for xWIndowsRestore, it jumps to a generic "browse code samples" webpage rather than a git-style site or other site with specifics on the module itself.

Proposed properties

Keep the current code as-is and merge into this module.

Special considerations or limitations

The addition of unit and integration tests would really be the only change to the module itself.

@PlagueHO
Copy link
Member

PlagueHO commented Apr 5, 2021

Linking this to dsccommunity/xWindowsRestore#5

Moving the resources over here would make sense as I don't think there is an official maintainer listed for this resource.

@johlju - what are your thoughts on merging xWindowsRestore to CompterManagementDsc? Would save you having to upgrade to new CI etc - however it looks a fair bit of work would need to be done on this to bring it up to HQRM -as there are no tests for the existing resources.

It does look like the resource is fairly well used (5K downloads per month).

@PlagueHO PlagueHO added discussion The issue is a discussion. resource proposal The issue is proposing a new resource in the resource module. labels Apr 5, 2021
@johlju
Copy link
Member

johlju commented Apr 5, 2021

I suggested in the (now archived) PowerShell/DscResources#374 that we move it here, so I'm happy with moving it here. But the code should be change to follow guideline and it should have have proper testing (at least unit test, if it is not possible to make integration tests).

@cohdjn
Copy link
Contributor Author

cohdjn commented Apr 5, 2021

I'm willing to do the legwork assuming you guys are willing to help with noobie questions/problems I run across?

@johlju
Copy link
Member

johlju commented Apr 5, 2021

@cohdjn Awesome, and absolutely, just ping us if you have any issues 🙂 You can reach us (and the rest of the Community) on Slack as well https://dsccommunity.org/community/contact/

@cohdjn
Copy link
Contributor Author

cohdjn commented Apr 16, 2021

@johlju Do you see any reason to bring the SystemRestorePoint resource over?

@johlju
Copy link
Member

johlju commented Apr 16, 2021

I’m not familiar with the resources, so if we turn around the question and I ask; Since you think it is not worth to bringover, what is the reason for not bring it over? 😄 is it obsolete?

@cohdjn
Copy link
Contributor Author

cohdjn commented Apr 16, 2021

I don't see how anything worthwhile is accomplished trying to create a system restore point every time the LCM runs. Generally speaking, Windows will take a restore point automatically when applications/drivers are installed or Windows Updates are applied. Users/administrators can manually create restore points too when they choose.

It's not an issue of obsolesce. It's an issue of using DSC in ways that it probably shouldn't be (my opinion). The adage of "just because you can doesn't mean you should" runs through my mind when I think about this. If you disagree, I'm more than happy to bring it over... I just don't want to waste time if you and/or @PlagueHO agreed there was little to no value.

@kilasuit
Copy link

@cohdjn you would have it as part of your usual config but may use it as a stepping stone config that you apply only as part of your patching cycle (not saying this is how it may being used but how I'd potentially use it)

@cohdjn
Copy link
Contributor Author

cohdjn commented Apr 16, 2021

@kilasuit So you see value having it available?

@kilasuit
Copy link

the xWindowsRestore module has had 130,022 downloads since v1, which granted was in 2014 so may be in use still.

@cohdjn
Copy link
Contributor Author

cohdjn commented Apr 16, 2021

Fair enough. I'll bring it across.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion The issue is a discussion. resource proposal The issue is proposing a new resource in the resource module.
Projects
None yet
4 participants