Copilot commented on code in PR #925:
URL: https://github.com/apache/incubator-graphar/pull/925#discussion_r3264035635
##########
cpp/CMakePresets.json:
##########
@@ -0,0 +1,66 @@
+{
+ "version": 4,
+ "cmakeMinimumRequired": {
+ "major": 3,
+ "minor": 21,
+ "patch": 0
+ },
+ "configurePresets": [
+ {
+ "name": "base",
+ "hidden": true,
+ "binaryDir": "${sourceDir}/build",
Review Comment:
Both `debug` and `release` presets inherit the same `binaryDir`
(`${sourceDir}/build`). Reconfiguring the same build directory with a different
`CMAKE_BUILD_TYPE` can produce mixed artifacts and confusing results; use
separate build dirs per preset (e.g., `build/debug` and `build/release`, or
`${sourceDir}/build/${presetName}`) to keep configurations isolated.
##########
cpp/README.md:
##########
@@ -88,35 +88,22 @@ cmake -DCMAKE_BUILD_TYPE=Debug -DBUILD_TESTS=ON ..
make -j8 # if you have 8 CPU cores, otherwise adjust, use -j`nproc` for
all cores
```
-### Quick Build with Scripts
+### Quick Build with CMake Presets
-For convenience, we provide build scripts for Ubuntu and macOS that configure
the following CMake options:
+We use CMake Presets for quick building. Available presets:
-- `-DBUILD_BENCHMARKS=ON`: Build benchmark executables
-- `-DCMAKE_BUILD_TYPE=Debug`: Build in debug mode
-- `-DBUILD_TESTS=ON`: Build unit tests
-- `-DBUILD_EXAMPLES=ON`: Build example executables
+- `debug` - Debug build with tests, examples, and benchmarks
+- `release` - Release build with tests, examples, and benchmarks
-**Ubuntu:**
```bash
-./build_ubuntu.sh
-```
-
-**macOS:**
-```bash
-./build_macos.sh
-```
+# Configure the project
+cmake --preset debug # or: cmake --preset release
-These scripts will automatically create the build directory, configure with
CMake, and compile the project. Build logs will be saved to `build_ubuntu.log`
or `build_macos.log`.
-
-After building, you can run the unit tests with:
-
-```bash
-git submodule update --init --recursive # download the testing data
-GAR_TEST_DATA=${PWD}/testing ctest
+# Build the project
+cmake --build --preset debug # or: cmake --build --preset release
```
Review Comment:
This section removed the prior instructions for fetching test data and
running tests. Since the PR adds `testPresets`, the quick-build docs should
include the corresponding `git submodule update --init --recursive` (if still
required) and `ctest --preset <name>` commands so users can validate their
build.
##########
cpp/CMakePresets.json:
##########
@@ -0,0 +1,66 @@
+{
+ "version": 4,
+ "cmakeMinimumRequired": {
+ "major": 3,
+ "minor": 21,
+ "patch": 0
+ },
+ "configurePresets": [
+ {
+ "name": "base",
+ "hidden": true,
+ "binaryDir": "${sourceDir}/build",
+ "cacheVariables": {
+ "CMAKE_GENERATOR": "Ninja",
+ "CMAKE_EXPORT_COMPILE_COMMANDS": "ON",
+ "BUILD_TESTS": "ON",
+ "BUILD_EXAMPLES": "ON",
+ "BUILD_BENCHMARKS": "ON"
Review Comment:
`CMAKE_GENERATOR` is being set as a cache variable, but CMake presets select
the generator via the top-level `generator` field (or via `--preset` defaults).
As written, this likely has no effect and may leave users building with an
unintended generator; move this to `"generator": "Ninja"` (and remove the cache
var) or drop the generator pin entirely.
##########
cpp/README.md:
##########
@@ -88,35 +88,22 @@ cmake -DCMAKE_BUILD_TYPE=Debug -DBUILD_TESTS=ON ..
make -j8 # if you have 8 CPU cores, otherwise adjust, use -j`nproc` for
all cores
```
-### Quick Build with Scripts
+### Quick Build with CMake Presets
-For convenience, we provide build scripts for Ubuntu and macOS that configure
the following CMake options:
+We use CMake Presets for quick building. Available presets:
Review Comment:
The preset-based workflow implicitly depends on a minimum CMake version (per
`CMakePresets.json`) and likely a specific generator (e.g., Ninja). The README
section should explicitly call out these prerequisites (CMake >= 3.21, and
installing Ninja if the presets use it) so users don’t hit immediate configure
failures.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
