Skip to content

CMake: Remove MUSE_COMPILE_BUILD_64 option that does nothing#49

Merged
igorkorsukov merged 2 commits into
musescore:mainfrom
cbjeukendrup:remove-build64-option
May 23, 2026
Merged

CMake: Remove MUSE_COMPILE_BUILD_64 option that does nothing#49
igorkorsukov merged 2 commits into
musescore:mainfrom
cbjeukendrup:remove-build64-option

Conversation

@cbjeukendrup
Copy link
Copy Markdown
Contributor

And remove cmake_wrapper.bat completely, as it is not used by this repo: it is a helper for MuseScore Studio’s build.cmake script.

And remove cmake_wrapper.bat completely, as it is not used by this repo: it is a helper for MuseScore Studio’s `build.cmake` script.
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 23, 2026

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 158c0f9f-ae2b-4b46-9542-52772d354060

📥 Commits

Reviewing files that changed from the base of the PR and between 8b0f8a8 and bd77920.

📒 Files selected for processing (4)
  • buildscripts/cmake/SetupBuildEnvironment.cmake
  • buildscripts/tools/cmake_wrapper.bat
  • framework/cloud/musescorecom/musescorecomservice.cpp
  • framework/cmake/MuseDeclareOptions.cmake
💤 Files with no reviewable changes (3)
  • buildscripts/tools/cmake_wrapper.bat
  • framework/cmake/MuseDeclareOptions.cmake
  • buildscripts/cmake/SetupBuildEnvironment.cmake

📝 Walkthrough

Walkthrough

This pull request removes the MUSE_COMPILE_BUILD_64 CMake build option and cleans up dependent configuration. The option definition in MuseDeclareOptions.cmake is deleted, and the MinGW-specific linker flag conditional on that option is removed from SetupBuildEnvironment.cmake, with _UNICODE UNICODE compile definitions added to that block. Additionally, a lambda closure in an async callback in musescorecomservice.cpp is corrected to explicitly capture this, ensuring proper closure semantics.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description is minimal and lacks the required template sections including issue reference, CLA confirmation, commit verification, and testing confirmation. Add required template sections: issue reference (Resolves: #NNNNN), CLA checkbox completion, commit message verification, and testing confirmation.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the main change: removing the MUSE_COMPILE_BUILD_64 CMake option that is non-functional.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@igorkorsukov igorkorsukov merged commit 6f508fc into musescore:main May 23, 2026
3 checks passed
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