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

Fix AudioClip I/O #1108

Merged
merged 13 commits into from
May 5, 2020
Merged

Fix AudioClip I/O #1108

merged 13 commits into from
May 5, 2020

Conversation

MaigoAkisame
Copy link
Contributor

@MaigoAkisame MaigoAkisame commented Mar 25, 2020

The existing code does not read and write audio signals without changing them.
I've fixed some of the related code, and added a test_audioclip_io test to ensure the audio does not change during I/O.

  • I have provided code that clearly demonstrates the bug and only works correctly when used with this PR
  • I have added suitable tests to the test suite in tests/
  • I have properly documented new or changed features in the documention or in the docstrings
  • I have properly documented unusual changes to the code in the comments around it
  • I have made note of any breaking/backwards incompatible changes

@coveralls
Copy link

Coverage Status

Coverage increased (+0.5%) to 65.24% when pulling 622cbda on MaigoAkisame:master into 710c6df on Zulko:master.

@coveralls
Copy link

coveralls commented Mar 25, 2020

Coverage Status

Coverage increased (+1.9%) to 66.756% when pulling a696e13 on MaigoAkisame:master into 6057cf0 on Zulko:master.

@tburrows13 tburrows13 added the bug-fix For PRs and issues solving bugs. label Mar 25, 2020
@tburrows13 tburrows13 changed the base branch from master to v2 March 25, 2020 13:11
@tburrows13 tburrows13 changed the base branch from v2 to master March 26, 2020 12:37
@MaigoAkisame
Copy link
Contributor Author

All checks have passed!

What's the next step to have this merged?

(It's my first time to make a pull request, so forgive me if I'm asking anything stupid)

@tburrows13
Copy link
Collaborator

Hey @MaigoAkisame thanks so much for making this PR! You've done everything you need to for now, it is just waiting for a maintainer to check that we're happy with it and then we can merge it into the master branch.

@MaigoAkisame
Copy link
Contributor Author

Thank you!

tests/test_AudioClips.py Outdated Show resolved Hide resolved
@MaigoAkisame
Copy link
Contributor Author

There has been no activity for almost 2 weeks. Is someone gonna merge this?

@tburrows13 tburrows13 added this to the Release v2.0.0 milestone Apr 15, 2020
@tburrows13
Copy link
Collaborator

Closes #1174. Thanks

@tburrows13
Copy link
Collaborator

Sorry, my mistake. Please merge the changes back in at MaigoAkisame#1.

@MaigoAkisame
Copy link
Contributor Author

Is that something I need to do? What do I need to do exactly?

@tburrows13
Copy link
Collaborator

tburrows13 commented May 4, 2020

Please just merge MaigoAkisame#1, and I think that I can sort the rest out after that. This is totally my fault, but I can't work out how to fix it without you doing that.

@MaigoAkisame
Copy link
Contributor Author

Sure! I just did it.

@tburrows13 tburrows13 reopened this May 4, 2020
@tburrows13
Copy link
Collaborator

Great, I'll merge when the tests finish. You'll be happy to know that this PR fixed #1174 perfectly.

@tburrows13 tburrows13 merged commit 8df153f into Zulko:master May 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug-fix For PRs and issues solving bugs.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants