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

Update windows-rs to 0.58 #346

Merged
merged 1 commit into from
Aug 18, 2024
Merged

Update windows-rs to 0.58 #346

merged 1 commit into from
Aug 18, 2024

Conversation

sagudev
Copy link
Member

@sagudev sagudev commented Jul 29, 2024

Fix #345.

My first time working with windows-rs, so review carefully. I tried to stick to the current code schematic, but the whole codebase would need a cleanup (a lot of clippy warnings).

Draft because I want a companion PR in servo.

@atouchet
Copy link
Contributor

Servo now has windows 0.58 in its dependencies. Does this still need to remain a draft?

@sagudev sagudev marked this pull request as ready for review August 18, 2024 10:27
@sagudev
Copy link
Member Author

sagudev commented Aug 18, 2024

Servo now has windows 0.58 in its dependencies. Does this still need to remain a draft?

Not really, I just forgot.

Copy link
Member

@mrobinson mrobinson left a comment

Choose a reason for hiding this comment

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

I am by no means an expert in the Win32 API, but looked over this a couple times and I don't see any obvious issues. If it works, I say let's go for it.

@jdm jdm added this pull request to the merge queue Aug 18, 2024
Merged via the queue into servo:main with commit b16bee1 Aug 18, 2024
16 checks passed
@sagudev
Copy link
Member Author

sagudev commented Aug 18, 2024

I am by no means an expert in the Win32 API, but looked over this a couple times and I don't see any obvious issues. If it works, I say let's go for it.

The problem is that even that if it does not works the bugs are subtile (most changes are in error handling which happens rarely, and we only run smoketest for win, we need windows-wpt).

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.

Update windows-rs
4 participants