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

extensions: Expose NAME via reexport instead of const #887

Merged
merged 1 commit into from
Mar 25, 2024

Conversation

MarijnS95
Copy link
Collaborator

Now that hand-written extension wrappers are exposed as modules instead of structs it is no longer needed to expose NAME as associated const when a more-concise pub use reexport will do.


I'm still ambivalent what to do about NAME. Time and time again the question comes up why extension X isn't available in ash::extensions::, and we have to explain that this is only for hand-writen extensions for which no wrapepr has been written yet, or is more often than not a command-less extension that does not need a wrapper.
Veterans will know that ash::extensions:: is only for those hand-written wrappers - and NAME is re-exposed from ash::vk:: there purely for convenience - but anyone else seems to expect all extensions to be available there. Not to mention that my extension lists in various apps look funky when mixed with ash::extensions::XXX::name() and ash::vk::XXXFn::name() (old 0.37 syntax).
This was also brought up recently in different form (citation needed): why not combine the autogenerated ExtensionFn with the hand-written wrapper (effectively have the handwritten wrapper extend it)? Regardless of if/how/when we implement that, such an approach could also eliminate having multiple NAMEs in different places.

Now that hand-written extension wrappers are exposed as `mod`ules
instead of `struct`s it is no longer needed to expose `NAME` as
associated `const` when a more-concise `pub use` reexport will do.
@MarijnS95 MarijnS95 force-pushed the extensions-name-reexport branch from 24a2f8e to 291402c Compare March 25, 2024 15:43
@MarijnS95 MarijnS95 merged commit ac8556e into master Mar 25, 2024
20 checks passed
@MarijnS95 MarijnS95 deleted the extensions-name-reexport branch March 25, 2024 20:02
@MarijnS95 MarijnS95 added this to the Ash 0.38 milestone Dec 3, 2024
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