Conversation
|
Should I merge it? |
I am afraid not. Want to have more test and script optimization before merge. |
Sounds good. I added |
|
I see. Parallel compilation helps a lot. Want to try Ninja after witnessing a lot of modern projects use it. And if our codebase continuously grows it may become more useful. |
There was a problem hiding this comment.
Pull Request Overview
This PR switches the CMake generator and build tool from Make to Ninja across all scripts, CI workflows, and example READMEs to improve build performance.
- Add
-G Ninjato CMake invocations - Replace
makeandcmake --buildcalls withninja - Install
ninja-build(orninja) in setup scripts and CI jobs
Reviewed Changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/setup_hooks.sh | Configure temporary lint build with Ninja |
| scripts/install_libcachesim.sh | Use Ninja instead of make |
| scripts/install_dev_dependency.sh | Install ninja-build/ninja packages |
| scripts/install_dependency.sh | Add Ninja to Ubuntu/Homebrew installs |
| scripts/debug.sh | Build debug target with Ninja |
| example/*/README.md | Update example build steps to use Ninja |
| README.md | Change root build instructions to Ninja |
| .github/workflows/*.yml | Install Ninja in CI and use ninja |
Comments suppressed due to low confidence (2)
README.md:124
- Since the build instructions now rely on Ninja, consider adding a prerequisite note instructing users to install Ninja (e.g.,
sudo apt install ninja-buildorbrew install ninja).
cmake -G Ninja .. && ninja
example/plugin_v2/README.md:18
- Add a brief prerequisite section or comment to remind users to install Ninja before running these commands.
cmake -G Ninja ..
|
LGTM |
For a faster build speed, propose to use Ninja as the generator of cmake.
Build.yml time cost: 2m49s -> 2m25s