Skip to content

Conversation

@byrnedj
Copy link
Contributor

@byrnedj byrnedj commented Sep 10, 2025

No description provided.

@byrnedj byrnedj requested a review from Copilot September 10, 2025 18:56
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR migrates the build system from a traditional Makefile to CMake for better cross-platform compatibility and modern build management. The change simplifies the build process while maintaining all existing functionality and adding proper package configuration for integration with other CMake projects.

  • Replaces Makefile-based build with CMake configuration
  • Adds CMake package configuration files for easier project integration
  • Updates documentation to reflect new build process

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
CMakeLists.txt Main CMake configuration defining library targets, dependencies, and installation rules
DTOConfig.cmake.in CMake package configuration template for project integration
dto.h New header file with C++ compatibility guards (currently empty API placeholder)
README.md Updated build instructions to use CMake instead of direct make commands
Makefile Removed legacy Makefile in favor of CMake

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@@ -0,0 +1,67 @@
cmake_minimum_required(VERSION 3.0)
Copy link

Copilot AI Sep 10, 2025

Choose a reason for hiding this comment

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

[nitpick] CMake 3.0 is quite old (from 2014). Consider requiring a more recent version like 3.16 or 3.20 for better modern CMake features and practices.

Suggested change
cmake_minimum_required(VERSION 3.0)
cmake_minimum_required(VERSION 3.16)

Copilot uses AI. Check for mistakes.
Comment on lines +26 to +28
check_c_compiler_flag("-mwaitpkg" HAS_WAITPKG)
if (HAS_WAITPKG)
target_compile_options(dto PRIVATE -mwaitpkg -march=native)
Copy link

Copilot AI Sep 10, 2025

Choose a reason for hiding this comment

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

The -march=native flag makes the binary non-portable as it optimizes for the build machine's CPU. Consider making this optional or using a more portable approach for distribution builds.

Suggested change
check_c_compiler_flag("-mwaitpkg" HAS_WAITPKG)
if (HAS_WAITPKG)
target_compile_options(dto PRIVATE -mwaitpkg -march=native)
# Option to enable -march=native (default OFF for portability)
option(DTO_USE_MARCH_NATIVE "Enable -march=native for best performance on local machine (non-portable)" OFF)
check_c_compiler_flag("-mwaitpkg" HAS_WAITPKG)
if (HAS_WAITPKG)
if (DTO_USE_MARCH_NATIVE)
target_compile_options(dto PRIVATE -mwaitpkg -march=native)
else()
target_compile_options(dto PRIVATE -mwaitpkg)
endif()

Copilot uses AI. Check for mistakes.
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.

1 participant