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

FifoBuffer adjustments #5803

Merged
merged 3 commits into from
Jan 23, 2021
Merged

FifoBuffer adjustments #5803

merged 3 commits into from
Jan 23, 2021

Conversation

M374LX
Copy link
Contributor

@M374LX M374LX commented Nov 24, 2020

The template in fifoBuffer is unnecessary because only one type is used (surroundSampleFrame *). The PR also fixes the coding conventions for that header file.

Additionally, it fixes the typoed method name Song::setLoadOnLauch().

@LmmsBot
Copy link

LmmsBot commented Nov 24, 2020

🤖 Hey, I'm @LmmsBot from github.com/lmms/bot and I made downloads for this pull request, click me to make them magically appear! 🎩

Linux

Windows

macOS

🤖
{"platform_name_to_artifacts": {"Linux": [{"artifact": {"title": {"title": "(AppImage)", "platform_name": "Linux"}, "link": {"link": "https://11893-15778896-gh.circle-artifacts.com/0/lmms-1.3.0-alpha.1.61%2Bg691d162-linux-x86_64.AppImage"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/11893?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}], "Windows": [{"artifact": {"title": {"title": "64-bit", "platform_name": "Windows"}, "link": {"link": "https://11890-15778896-gh.circle-artifacts.com/0/lmms-1.3.0-alpha.1.61%2Bg691d16276-mingw-win64.exe"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/11890?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}, {"artifact": {"title": {"title": "32-bit", "platform_name": "Windows"}, "link": {"link": "https://11894-15778896-gh.circle-artifacts.com/0/lmms-1.3.0-alpha.1.61%2Bg691d16276-mingw-win32.exe"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/11894?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}, {"artifact": {"title": {"title": "32-bit", "platform_name": "Windows"}, "link": {"link": "https://ci.appveyor.com/api/buildjobs/fyyyd2wfmhxjsld0/artifacts/build/lmms-1.3.0-alpha-msvc2017-win32.exe"}}, "build_link": "https://ci.appveyor.com/project/Lukas-W/lmms/builds/37054157"}, {"artifact": {"title": {"title": "64-bit", "platform_name": "Windows"}, "link": {"link": "https://ci.appveyor.com/api/buildjobs/3u42qlbs3vk6re9u/artifacts/build/lmms-1.3.0-alpha-msvc2017-win64.exe"}}, "build_link": "https://ci.appveyor.com/project/Lukas-W/lmms/builds/37054157"}], "macOS": [{"artifact": {"title": {"title": "", "platform_name": "macOS"}, "link": {"link": "https://11891-15778896-gh.circle-artifacts.com/0/lmms-1.3.0-alpha.1.61%2Bg691d16276-mac10.14.dmg"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/11891?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}]}, "commit_sha": "c65c5dd4e2f6af05f44b65d22e876d40154cf392"}

@Veratil
Copy link
Contributor

Veratil commented Nov 24, 2020

I like the updates, but I don't think removing the template is necessary even if we're only using it with a single type.

@JohannesLorenz
Copy link
Contributor

@M374LX Can you please think about the fix from @Veratil (and fix the conflict by a merge or rebase)?

@M374LX
Copy link
Contributor Author

M374LX commented Dec 30, 2020

Rebase done.

Should I keep the removed template? If so, this will become just another reformatting PR.

@Veratil
Copy link
Contributor

Veratil commented Dec 30, 2020

Should I keep the removed template? If so, this will become just another reformatting PR.

I think keeping is better for now.

@PhysSong
Copy link
Member

PhysSong commented Jan 8, 2021

Additionally, it fixes the typoed method name Song::setLoadOnLauch().

I think that shouldn't be in a PR which changes Mixer-related stuff?

@M374LX
Copy link
Contributor Author

M374LX commented Jan 8, 2021

@PhysSong It is just a typo I noticed while preparing this PR.

@JohannesLorenz
Copy link
Contributor

This should be merged before reorg, so I'm reviewing this now.

@JohannesLorenz JohannesLorenz self-requested a review January 23, 2021 12:30
Copy link
Contributor

@JohannesLorenz JohannesLorenz left a comment

Choose a reason for hiding this comment

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

LGTM

@JohannesLorenz JohannesLorenz merged commit e900576 into LMMS:master Jan 23, 2021
@M374LX M374LX deleted the fifobuffer branch January 24, 2021 03:06
IanCaio pushed a commit to IanCaio/lmms that referenced this pull request Mar 28, 2021
devnexen pushed a commit to devnexen/lmms that referenced this pull request Apr 10, 2021
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.

5 participants