Skip to content

Conversation

@awcullen
Copy link

First try...

@awcullen
Copy link
Author

awcullen commented Jul 2, 2018

I see that Partition also allows you pass in a slice of Buffers with the result being one big logical buffer. The wrinkle is that the passed in buffers may be of varying lengths. In this case I believe WriteAt should not be able to lengthen any buffer except for the last. Let me fix this case and add a test.

@djherbis
Copy link
Owner

djherbis commented Jul 2, 2018

Hey, I'll review this when I get chance, but is there a reason you chose to continue down this route vs. just using something like io.MultiReader etc. mentioned in #7.

@djherbis
Copy link
Owner

djherbis commented Jul 4, 2018

First few comments:

  • Don't commit debug.test
  • Instead of adding the *At versions into the existing files, can we copy the existing files, add _at to them (except the test file), it'll make it easier for me to diff the files (I'm expecting the diff to basically just be in types except for ReadAt/WriteAt, and tests)
  • Please reference the issue in the commit message Partition that implements BufferAt #7
  • In your tests, please make sure you include the checkCap, runPerfectSeries tests as well

@janekolszak
Copy link

Hi,
Are you planning to merge?
Thanks

@djherbis
Copy link
Owner

Hey @janekolszak I'm editing the PR now, I've made some progress applying the changes I originally requested.

Once I've finished tidying it up I'll merge it.

@janekolszak
Copy link

Do you need any help?

@djherbis
Copy link
Owner

djherbis commented Aug 30, 2019 via email

@djherbis djherbis merged commit 1ad28b1 into djherbis:master Sep 2, 2019
@djherbis
Copy link
Owner

djherbis commented Sep 2, 2019

@janekolszak Merged it in, along with my changes and tests

@janekolszak
Copy link

Thank you so much! :)

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