-
Notifications
You must be signed in to change notification settings - Fork 185
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
Adding BlobDataProvider for dynamically loaded data blobs #1084
Conversation
Design notes:
|
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.
Looks okay, I'm surprised this isn't causing ICEs
} | ||
} | ||
|
||
pub fn try_from_yoked_buffer<T, E>( |
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.
issue: docs
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.
(and elsewhere)
no need for examples, just useful to have a one-liner about what's going on
The ICEs are on stable but are fixed on beta. I can't check this in right now because of the ICEs. Did you have any thoughts on the two design points in my previous comment? |
@sffc oh, I didn't realize those were questions. (1) is basically exactly how Yoke is supposed to be used -- it doesn't matter if the cart is referencing a wider set of data since yoke doesn't care about the cart except for the destructor. (2) is also reasonable and is what I woudl expect here especially given the upcoming refactors |
Hooray! The files in the branch are the same across the force-push. 😃 ~ Your Friendly Jira-GitHub PR Checker Bot |
|
||
/// A version of [`Yoke::project_cloned`] that takes a capture and bubbles up an error | ||
/// from the callback function. | ||
pub fn try_project_cloned_with_capture<'this, P, T, E>( |
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.
_with_capture
should be unnecessary on newer Rust -- we should be able to get away with using the fn traits, but that's a followup we can try
This PR builds and runs against Rust 1.56 (October 21, 2021). Unfortunately it's not just the docs tests that don't build; the code itself doesn't build.