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

Replace winapi dependency with windows-sys #91

Merged
merged 3 commits into from
Jan 9, 2023

Conversation

Kijewski
Copy link
Collaborator

@Kijewski Kijewski commented Dec 13, 2022

Some time ago I spoke in favor of using winapi instead of
windows-sys, because it had fewer msrv bumps, and because it was
used by e.g. tokio, and chrono, so even if it was not being
developed for anymore, it still had a ton of users who would hopefully
notice any bugs.

By now tokio has switched to windows-sys, and chrono will too in
the next release. I guess it's time then to make the switch, too.

@Kijewski Kijewski added dependencies Pull requests that update a dependency file Unsafe-Proposed `unsafe` code is added or changed Tier-1 Rust Tier-1 platform labels Dec 14, 2022
@Kijewski Kijewski changed the title Use windows-sys Replace winapi dependency with windows-sys Dec 14, 2022
@Kijewski Kijewski force-pushed the pr-windows-sys branch 5 times, most recently from 743d01c to bc63d1d Compare December 14, 2022 16:17
[Some time ago] I spoke in favor of using [`winapi`] instead of
[`windows-sys`], because it had fewer msrv bumps, and because it was
used by e.g. `tokio`, and `chrono`, so even if it was not being
developed for anymore, it still had a ton of users who would hopefully
notice any bugs.

By now `tokio` has switched to `windows-sys`, and `chrono` will too in
the next release. I guess it's time then to make the switch, too.

[Some time ago]: strawlab#35
[`winapi`]: https://crates.io/crates/winapi
[`windows-sys`]: https://crates.io/crates/windows-sys
Copy link
Member

@astraw astraw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I checked that this runs on my Windows machine and looked through it for a few minutes. I don't see any obvious problems. Although I'm certainly no expert in the relevant APIs, I will approve this now but would welcome another look by any interested party.

@astraw astraw merged commit 94d2e79 into strawlab:main Jan 9, 2023
@astraw
Copy link
Member

astraw commented Jan 9, 2023

Thanks. Sorry this took a while to review.

@djc
Copy link
Contributor

djc commented Mar 22, 2023

So chrono 0.4 is still the active release branch, and is currently still maintaining an MSRV of 1.38. We're working on 0.5 (which requires 1.48) but that's not very close to being done. Would it be a big ask to request that this project keeps its MSRV at 1.38 for a bit longer?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file Tier-1 Rust Tier-1 platform Unsafe-Proposed `unsafe` code is added or changed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants