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

Remove the ash::vk stutter #2640

Merged
merged 1 commit into from
Feb 7, 2025
Merged

Conversation

marc0246
Copy link
Contributor

@marc0246 marc0246 commented Feb 6, 2025

ash::vk is a module because it's supposed to be imported. It's really hard to look at whatever this was.

@Rua
Copy link
Contributor

Rua commented Feb 7, 2025

I don't see why you think it was supposed to be imported. But I do think this is a good change regardless.

@Rua Rua merged commit 478bad3 into vulkano-rs:master Feb 7, 2025
6 checks passed
@marc0246 marc0246 deleted the ash-vk-stutter branch February 7, 2025 15:53
@marc0246
Copy link
Contributor Author

marc0246 commented Feb 7, 2025

I say that because that's how the crate is designed and how everyone uses it. Instead of writing VkDeviceCreateInfo you write vk::DeviceCreateInfo. That's why the module is called vk and the types inside don't have the Vk prefix. Typing ash::vk every time is too much syntactic salt and there's no reason for it. On the other hand, there is no reason to import the types inside, as it's a flat module with no other modules on the inside, and the sheer number of types that you would need to import. You aren't supposed to import the types inside even if there were no naming conflicts.

Not every crate is designed this way. vulkano for one doesn't have a flat module structure. It's extremely deeply nested in cases. Also, the module names are quite long. You are pretty much forced to import the types. This is IMO a very big deficiency, given as mentioned, the sheer amount of types that you need. Then there are also crates that have everything at the top level, where you aren't meant to import anything, such as wgpu and many linear algebra libraries.

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.

2 participants