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

chore: relax a bunch of Sync requirements where unnecessary #33

Merged
merged 2 commits into from
Dec 5, 2024

Conversation

miraclx
Copy link
Contributor

@miraclx miraclx commented Dec 15, 2023

Linked: ferristseng/rust-ipfs-api#140

I noticed a weird trait restriction in https://github.com/ferristseng/rust-ipfs-api that made it so only streams implementing Sync are permitted to the upload API. Even though the download API doesn't abide by those same rules.

Effectively makes it so you can't pipe the output of, say.. cat into add. Because add expects more than cat can offer.

And this will be the case most of the time. I looked under the hood, and couldn't find any reasons why the stream itself needs to be Sync.

So this PR gets rid of all the Sync bounds where they are present.

I've also tested it downstream with the ipfs-api repo and in my own project as well, and it all checks out.

@ferristseng
Copy link
Owner

Thanks! This looks great.

@ferristseng ferristseng merged commit 8e825fa into ferristseng:main Dec 5, 2024
1 check passed
@miraclx miraclx deleted the relax-sync-requirement branch December 5, 2024 14:46
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