ohadravid / wmi-rs

WMI crate for rust
Apache License 2.0
84 stars 27 forks source link

clippy and fmt changes #64

Closed sn99 closed 1 year ago

sn99 commented 1 year ago

I was using your crate to make a wrapper library and thought about migrating towindows-rs from winapi. This pull is precursor to it.

Although this test seems to be failing even before I made any changes

test de::wbem_class_de::tests::it_desr_into_map ... FAILED

failures:

---- de::wbem_class_de::tests::it_desr_into_map stdout ----
thread 'de::wbem_class_de::tests::it_desr_into_map' panicked at 'assertion failed: langs.contains(&Variant::String(system_lang))', src\de\wbem_class_de.rs:302:42
ohadravid commented 1 year ago

Thanks @sn99 😁

You wrapper is a very nice idea! Defining the actual WMI objects in Rust is finicky, but IMO for a lot of usecases just a HashMap<String, Variant> is not very useful, so maybe consider an API which could be later extended to use automatic MOF/MSDN format -> Rust fields conversion, and maybe support for a user-provided connection?

I think migrating to windows-rs would be really awesome. If you continue working on it, try to have the work split into manageable PRs?

sn99 commented 1 year ago

That's the plan! I've already begun working on a separate PR to migrate to windows-rs. Additionally, I'm looking to expand the appeal of the wrapper to a wider audience (currently, I've only proposed using it for office work instead of sysinfo). I'll revisit this PR in about a week or so, after my university midterms are over. Thanks!

ohadravid commented 1 year ago

FYI #67 / @vthib

vthib commented 1 year ago

Interesting ^^

I already have a working migration to windows locally, I just have to clean-it up before making a PR for it, so you may not need to spend too much time doing this migration yourself @sn99

sn99 commented 1 year ago

@vthib I will leave it to you then

sn99 commented 1 year ago

Closed in favor of #67