-
Notifications
You must be signed in to change notification settings - Fork 182
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
Use DataPayload::from_static_ref
in databake
#3500
Conversation
Any particular reason for killing the lookup method? It feels like it's still neater for diagnostics and debugging. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
r=me but i kinda liked the separate lookup method, it felt cleaner. We don't have to call it from other spots; it's fine even if it's called just once
(also: the lack of .data.rs for the macro files is kinda annoying, not opposed to that fix being rolled into this PR as well. I'm okay with macros.rs still being .rs)
It simplifies adding the fallback (#3487), because we need to put the resolved locale in the metadata. With an inlined |
Why is that annoying? They're still marked as generated files. |
Yeah but you can no longer make them go away completely in the github review view |
This allows inlining the lookup functions and
AnyProvider
to be backed byDataProvider
.I've also inlined the skeletons special case, as it's only called in one location now and I don't like having the hacky zerocopy on the API.
#2945