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

Separate analysis and synthesis #4

Draft
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

PieterPenninckx
Copy link
Contributor

This pull request separates the analysis from the synthesis. The purpose is to enable things like time stretching.

@nwoeanhinnogaehr
Copy link
Owner

Haven't had a chance to look at this in detail yet, but I'm fully supportive of this change! This would greatly expand the use cases. Thanks!

@PieterPenninckx
Copy link
Contributor Author

Thank you for your kind words and for reviewing my pull requests!

I would like to try out the proposed API myself in order to test it and this may take some time, but I got distracted by other things, so it may take a while longer. So here are some things we can do:

  • Just keep the PR open a little longer. There aren't many people contributing to this crate, so this probably doesn't block anybody.
  • Remove the pub keyword for the new structs. This would allow us to change the API later and then it wouldn't be a breaking change.

What do you think about it?

@nwoeanhinnogaehr
Copy link
Owner

My sincere apologies for forgetting about this! I'm thinking what would be best is either to keep the PR open, or to merge it onto a new branch. Then when things are done I can merge with the master branch. Up to you.

@PieterPenninckx
Copy link
Contributor Author

@nwoeanhinnogaehr , no worries. The limiting factor is the amount of time I have. I think we'll just leave the PR open. When I have validated that the API is ergonomic, I can let you know that it's ready for review and if I notice some usability problems, I can still push commits.

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