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

Breaking API Change: Swap indexes of Wav.Data*. Add API for reading WAV in chunks. Added unit tests for 'wav' package #3

Closed
wants to merge 1 commit into from

Conversation

nlefler
Copy link
Contributor

@nlefler nlefler commented Dec 24, 2013

Changes in this commit:

  1. Changed Wav.Data8, Wav.Data16, and Wav.Data to be indexed by sample and then by channel.
  2. Added API to stream a wav from an io.Reader rather than loading all data into memory.
  3. Added unit tests for 'wav' package.

Change #1 is a breaking change to the current API.

@maddyblue
Copy link
Owner

Sorry I've taken so long to review this. I'd ask that you split these changes up into separate PRs, though, so they are more reviewable. Regarding each change:

  1. What is the reason to change to sample-indexing instead of channel-indexing? Does it solve any problems or make more sense?
  2. Fantastic. Move this to its own PR and it'll get merged.
  3. I'm happy to see tests, but I'm not willing to use goconvey. It's a fine project that I've used before, but I prefer to stick to the stock tests API.

@nlefler
Copy link
Contributor Author

nlefler commented Jan 12, 2014

Sure I can do that. I'll include reasoning for the index change in it's PR. Closing this.

@nlefler nlefler closed this Jan 12, 2014
@nlefler nlefler deleted the chunked_read branch January 29, 2014 17:07
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