Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Coroutines Conversion: convert Blob::BlobInputStream #864

Merged
merged 3 commits into from
Jul 17, 2023

Conversation

jasnell
Copy link
Member

@jasnell jasnell commented Jul 14, 2023

As part of the effort to begin broader adoption of coroutines across the codebase, I plan to open a number of small examplar PRs as part of the preparation for the larger effort to devise and document a plan for the larger effort. This is largely intended to help tease out whether there are any particularly thorny issues that may be uncovered while we go through the process, but also to provide examples for the team on how the conversion can be done.

This PR picks one small API largely at random (Blob::BlobInputStream) that is easy to convert.

And since I'm in the code, I'm also taking the opportunity to do a bit of Drive By cleanup, documentation, and minor refactoring to simplify impl along the way.

Copy link
Member

@kentonv kentonv left a comment

Choose a reason for hiding this comment

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

The actual coroutine conversion looks good but we need to talk more about the comment style change. :)

src/workerd/api/blob.c++ Outdated Show resolved Hide resolved
src/workerd/api/blob.c++ Show resolved Hide resolved
@jasnell jasnell force-pushed the jsnell/coroutines-conversion-blobinputstream branch from 2c8458f to c466815 Compare July 17, 2023 14:42
@jasnell jasnell requested a review from kentonv July 17, 2023 14:44
@jasnell jasnell force-pushed the jsnell/coroutines-conversion-blobinputstream branch from c466815 to 1160fb2 Compare July 17, 2023 14:44
@jasnell jasnell merged commit 7967ac1 into main Jul 17, 2023
@fhanau fhanau deleted the jsnell/coroutines-conversion-blobinputstream branch December 4, 2023 22:20
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants