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 for the SF2 with no "default" patch #5084

Merged
merged 5 commits into from
Aug 13, 2019
Merged

Fix for the SF2 with no "default" patch #5084

merged 5 commits into from
Aug 13, 2019

Conversation

kdeng00
Copy link
Contributor

@kdeng00 kdeng00 commented Jul 20, 2019

Using the instructions from #323 to test the fix for this PR, I was able to reproduce the problem and provide a fix. Let me know if there is anything else that is left from the issue.

Copy link
Member

@PhysSong PhysSong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your work, and sorry for the late response.
Here are some feedbacks.

plugins/sf2_player/sf2_player.cpp Outdated Show resolved Hide resolved
plugins/sf2_player/sf2_player.cpp Show resolved Hide resolved
plugins/sf2_player/sf2_player.cpp Outdated Show resolved Hide resolved
@kdeng00
Copy link
Contributor Author

kdeng00 commented Jul 31, 2019

Hey @PhysSong,
Thanks for the feedback and I made the changes. However, there is one test that is failing for Mac OS X and I can't seem to understand why it is failing.

@zonkmachine
Copy link
Member

However, there is one test that is failing for Mac OS X and I can't seem to understand why it is failing.

I think it's Homebrew acting up a bit. Restarting the Mac build a couple of times worked.

@kdeng00
Copy link
Contributor Author

kdeng00 commented Aug 1, 2019

Sounds good, I would have thought that since Mac OS was POSIX compliant that it would have just been compatible but hombrew tool is something else.

@Reflexe Reflexe merged commit 00d2cd3 into LMMS:master Aug 13, 2019
sdasda7777 pushed a commit to sdasda7777/lmms that referenced this pull request Jun 28, 2022
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