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

[WIP] WMI adapter improvements #520

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

Gijsreyn
Copy link
Contributor

PR Summary

Starting to address #475 and new wmiAdapter module included

PR Context

@Gijsreyn Gijsreyn changed the title WMI adapter improvements [WIP] WMI adapter improvements Aug 15, 2024
@Gijsreyn
Copy link
Contributor Author

@anmenaga Hey Andrew, I definitely didn’t want to step on your toes, but while I was testing the WMI adapter, I thought I'd contribute a bit. Would you mind taking a look at my initial commit when you have a moment?

@Gijsreyn
Copy link
Contributor Author

Gijsreyn commented Sep 5, 2024

@michaeltlombardi Hey Mikey, I could not attend the DSC community call last week so was unable to ask the question here. In the wmi.resource.ps1 file, resources are listed in a [pscustomobject] matching the dsc-resource schema definition. When developing the set operation capability, I stumbled on the fact, that there is not a key property that allows setting the CimClassMethods

I don't know if this is something that can be changed. In the current PR, I coined to use methodDetails or methods. I recognize other adapters "might" require to implement it or set it as null / array value.

If it's something to be discussed into a discussion, then that's also fine.

@SteveL-MSFT
Copy link
Member

@Gijsreyn I appreciate you taking the time to help improve the WMI adapter, however, I think it would be more productive for us if you can make smaller targeted changes making it easier to review and get merged in rather than one large PR that will take a lot longer to review and iterate.

@Gijsreyn
Copy link
Contributor Author

@SteveL-MSFT Thanks for the response. I get the point, thought of picking most of the content what was already available from the powershell-adapter.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants