-
-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Rename FxMixer to Mixer #6239
Rename FxMixer to Mixer #6239
Conversation
Fix changing LcdSpinBox value changing their init value (LMMS#6241)
The code can not compile due to this: In Engine.h, you use Same issue in |
🤖 Hey, I'm @LmmsBot from github.com/lmms/bot and I made downloads for this pull request, click me to make them magically appear! 🎩
Windows
Linux
macOS🤖{"platform_name_to_artifacts": {"Windows": [{"artifact": {"title": {"title": "32-bit", "platform_name": "Windows"}, "link": {"link": "https://15475-15778896-gh.circle-artifacts.com/0/lmms-1.3.0-alpha.1.200%2Bge63e2f0bb-mingw-win32.exe"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/15475?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}, {"artifact": {"title": {"title": "64-bit", "platform_name": "Windows"}, "link": {"link": "https://15479-15778896-gh.circle-artifacts.com/0/lmms-1.3.0-alpha.1.200%2Bge63e2f0bb-mingw-win64.exe"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/15479?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/myrfnb4ivjumwf2q/artifacts/build/lmms-1.3.0-alpha-msvc2017-win32.exe"}}, "build_link": "https://ci.appveyor.com/project/Lukas-W/lmms/builds/42123895"}, {"artifact": {"title": {"title": "64-bit", "platform_name": "Windows"}, "link": {"link": "https://ci.appveyor.com/api/buildjobs/30336pb8yemdsbmu/artifacts/build/lmms-1.3.0-alpha-msvc2017-win64.exe"}}, "build_link": "https://ci.appveyor.com/project/Lukas-W/lmms/builds/42123895"}], "Linux": [{"artifact": {"title": {"title": "(AppImage)", "platform_name": "Linux"}, "link": {"link": "https://15478-15778896-gh.circle-artifacts.com/0/lmms-1.3.0-alpha.1.200%2Bge63e2f0bb-linux-x86_64.AppImage"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/15478?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}], "macOS": [{"artifact": {"title": {"title": "", "platform_name": "macOS"}, "link": {"link": "https://15477-15778896-gh.circle-artifacts.com/0/lmms-1.3.0-alpha.1.200%2Bge63e2f0bb-mac10.14.dmg"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/15477?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}]}, "commit_sha": "abf64c976adf50509318eed35368d534a70574db"} |
Co-authored-by: Hyunjin Song <tteu.ingog@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only 4 comments added.
Also, don't forget the stuff from Discord (e.g. the "Send to FX channel" should be "Send to Mixer channel", and the "FX" in InstrumentWindow should be "CH", ...).
Additional note: The submodule "doc/wiki" contains multiple occurrences of "fx", "Fx" or "FX". Those should be fixed, too. |
Co-authored-by: Johannes Lorenz <1042576+JohannesLorenz@users.noreply.github.com>
Co-authored-by: Johannes Lorenz <1042576+JohannesLorenz@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only 4 new comments, and the remaining wiki changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code is OK, except remaining wiki (Becoming-a-Theme-Developer.md, LMMS-Architecture.md)/GitBook changes.
Basic tests done, all OK.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review limitations:
- Only checked line diffs, no real bigger picture considerations
- Filtered out all changes to mmp, mpt, ts, and xpf files
An additional request: In the comments for the upgrade routines, please either change "savings" to "project files", or omit it entirely. I would also suggest moving all the upgrades into one function ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
@yashraj466 Spekular and me think that the PR is good. Do you want us to merge this, or do you still want to do more on this PR? |
Okey 👍 |
@yashraj466 Merged. Thanks a lot for your help! |
Welcome 😃 |
This PR aims to rename FxMixer to Mixer, as decided in #6089 and #5592.