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

expose winit #860

Merged
merged 1 commit into from
Feb 12, 2025
Merged

expose winit #860

merged 1 commit into from
Feb 12, 2025

Conversation

richard-uk1
Copy link
Contributor

I don't think there is a reason not to, and you need winit for some types so currently you must define the crate yourself, which risks version errors etc.

Copy link
Member

@DJMcNab DJMcNab left a comment

Choose a reason for hiding this comment

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

So, this is trying to solve one half of #584?

There are a couple of things to think about here:

  1. This is also an intermediate state, see this comment.
  2. There are reasonable use cases for using Xilem without winit

I think I'm happiest just leaving it in the state that you need to depend on winit for the moment, especially as you already need to depend on Masonry.

Copy link
Contributor

@PoignardAzur PoignardAzur left a comment

Choose a reason for hiding this comment

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

LGTM.

@PoignardAzur
Copy link
Contributor

PoignardAzur commented Feb 12, 2025

I think I'm happiest just leaving it in the state that you need to depend on winit for the moment, especially as you already need to depend on Masonry.

Sorry, I misread this sentence and thought you were approving.

My own perspective is that we'll eventually decouple Masonry from winit (having vocabulary crates for winit events would be a good first step) but until that happens, re-exporting it is fine, and is in line with how we treat other dependencies.

@richard-uk1 richard-uk1 added this pull request to the merge queue Feb 12, 2025
@richard-uk1
Copy link
Contributor Author

Yes if you decouple then you'd want to reverse this out.

Merged via the queue into main with commit b89d9b1 Feb 12, 2025
18 checks passed
@richard-uk1 richard-uk1 deleted the expose_winit branch February 12, 2025 22:34
eugenesvk pushed a commit to eugenesvk/xilem that referenced this pull request Feb 18, 2025
I don't think there is a reason not to, and you need winit for some
types so currently you must define the crate yourself, which risks
version errors etc.
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.

3 participants