-
Notifications
You must be signed in to change notification settings - Fork 4
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
Merge v1.18 from main #3
Conversation
For a detailed breakdown of changes, see #1
How are we proceeding here? The code looks like it would not break parameters from old save files, and aside from that, I assume that the author themselves reviewed their code. So maybe just a bit of testing? Should that be enough? |
Yes, I think so. I had promised Richard Furse to send him some tests specifically in regards to Freeverb and sample rate changes, but never got around to it. Quoting:
|
For clarification, we already had these sample rate changes in our fork. v1.18 upstreams them. |
OK. Then I guess that you don't need to be a DSP expert to test the changes? |
I would agree. |
OK. Then testing this would be most easy in LMMS/lmms#6151 (which has no build yet), correct? |
tested this branch, fix confirmed. LGTM. |
Merging based on @Rossmaxx's testing. |
Brings over 1.18 changes into the custom lmms branch.
Supersedes #2.
For a detailed breakdown of changes, see #1
Note, #1 has a lot of unnecessary whitespace changes, particularly
\r
\n
changes. These can be squashed out with a.gitattributes
file as described here: https://stackoverflow.com/a/26490990/3196753. This should be decided before merging (we can force-push these tomain
and then rebase this pull request).