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

Add io.ReaderAt interface compatibility #285

Merged
merged 3 commits into from
Aug 19, 2020
Merged

Conversation

qrpike
Copy link
Contributor

@qrpike qrpike commented Apr 11, 2019

Read is now a proxy for ReadAt. Read uses the returned bytes count to set the new offset for the file, since ReadAt does not alter the offset.

Read is now a proxy for ReadAt. Read uses the bytes read to set the new offset for the file, since ReadAt does not alter the offset.
client.go Show resolved Hide resolved
@eikenb eikenb self-assigned this May 23, 2019
@eikenb
Copy link
Member

eikenb commented Jun 12, 2019

I still think this might be possible to merge in.. I just have had zero time to test it out. I'll try to get to it as soon as I can find the time. Thanks.

@eikenb
Copy link
Member

eikenb commented Sep 29, 2019

Sorry for the long delay getting to this. It is still on my radar and I'm trying to make more time for this project. I hope to get to it soon. Thank you for your patience.

@eikenb eikenb removed their assignment Nov 10, 2019
@puellanivis
Copy link
Collaborator

This one might be good to get in for a 1.12 release.

@puellanivis puellanivis added this to the v1.12.0 milestone Jul 25, 2020
@nightlyone
Copy link

@puellanivis / @eikenb This looks like the last pull request that needs to be merged for a v1.12.0 release.

Would you mind doing that merge and then the release as well? I see no further work blocking the release.

@puellanivis
Copy link
Collaborator

So, I’m going to merge this, but also immediately start a new branch to add a mutex to protect the Read() call from multiple parallel calls.

It will be a lot safer than someone trying to figure out the inevitable race condition.

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.

4 participants