⚾ Basic functionality #2
Conversation
Bumps [actions/checkout](https://github.com/actions/checkout) from 3 to 4. - [Release notes](https://github.com/actions/checkout/releases) - [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md) - [Commits](actions/checkout@v3...v4) --- updated-dependencies: - dependency-name: actions/checkout dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <[email protected]>
…s/actions/checkout-4 Bump actions/checkout from 3 to 4
…rectly initializes.
…when a process has no override configuration.
| push: | ||
| pull_request: | ||
|
|
||
| jobs: |
There was a problem hiding this comment.
We should add a job that runs api_gen and compares the output, like some (but not all) of our other bindings crates.
(In other words you can copy-paste the setup from one of the others we have!)
| ``` | ||
|
|
||
| ```rust | ||
| // A code example |
| adapter_properties.Size = std::mem::size_of::<ctl_device_adapter_properties_t>() as u32; | ||
| adapter_properties.Version = 0; |
There was a problem hiding this comment.
There was a problem hiding this comment.
Field Record Update. Instead of your MaybeUninit assume_init() hack, something like:
let adapter_properties = ctl_device_adapter_properties_t {
Size: std::mem::size_of::<ctl_device_adapter_properties_t>() as u32,
Version: 0;
..Default::default() // or unsafe { mem::zeroed() }
};| // LUID is only available on windows. | ||
| #[cfg(target_os = "windows")] | ||
| let mut device_id = { | ||
| let luid_size = std::mem::size_of::<LUID>(); |
There was a problem hiding this comment.
So we only use the Windows type to get the size? Why don't we store (and pass a pointer to) an instance of LUID?
There was a problem hiding this comment.
Something to look into, to what extent we can unify the interfaces between platforms if multiple are supported. But it would also be an option to expose different types and functions depending on the platform of course. I suppose the caller would have to be aware either way to use the data sensibly.
| #[cfg(not(target_os = "windows"))] | ||
| let device_id = vec![]; |
There was a problem hiding this comment.
You can also wrap the existence of this item in the struct with cfg(windows).
There was a problem hiding this comment.
I was thinking about doing that, but I'm not sure what I prefer. I'm also not sure if this library even runs outside of Windows environments at all.
There was a problem hiding this comment.
If we don't intend that we shouldn't have the cfgs 😉
(And instead have the cfg in our own crate to disable this dependency)
src/lib.rs
Outdated
| adapter_properties.device_id_size = luid_size as u32; | ||
| device_id | ||
| }; | ||
| #[cfg(target_os = "windows")] |
There was a problem hiding this comment.
We'll have to find a cleaner solution for this, but won't fix it now.
There was a problem hiding this comment.
What do you mean? It's just more common to use cfg(windows) over cfg(target_os = "windows").
This does a few things: