Skip to content

Conversation

@DNKpp
Copy link

@DNKpp DNKpp commented Jan 25, 2026

Hello,
I'm currently reviewing whether I can adapt reflect-cpp for the project I'm working on. Unfortunately, the integration does not work as of now, because I'm already depending on enchantum, thus there is an existing integraiton. As reflect-cpp brings its own copy under the thirdparty folder, that results in lots of clashes when including rfl.hpp.
There already is the option REFLECTCPP_USE_BUNDLED_DEPENDENCIES which does switch whether the thirdparty folder is set as include directory, and thus conditionally includes the bundled ctre and yyjson, or an external version.

Currently, enchantum includes are always prefixed with the subfolder thirdparty, which prevents this mechanism to work.

@gemini-code-assist
Copy link

Summary of Changes

Hello @DNKpp, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request resolves a critical integration issue where reflect-cpp's bundled enchantum library caused conflicts with projects that already had enchantum installed. By modifying the build system and internal include paths, reflect-cpp can now consistently leverage an external enchantum dependency when configured to do so. This enhancement significantly improves reflect-cpp's compatibility and reduces friction when incorporating it into existing C++ ecosystems.

Highlights

  • CMake Configuration Update: The CMake configuration has been updated to correctly find and link an external enchantum library when the DREFLECTCPP_USE_BUNDLED_DEPENDENCIES option is active, ensuring reflect-cpp can integrate with projects already using enchantum.
  • Enchantum Include Path Refactoring: All direct include paths for enchantum across various reflect-cpp header files have been modified to remove the thirdparty/ prefix. This change allows reflect-cpp to use either its bundled enchantum or an external version based on the build configuration, resolving potential header conflicts.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@DNKpp DNKpp changed the title Make DREFLECTCPP_USE_BUNDLED_DEPENDENCIES option consistent with enchantum Make REFLECTCPP_USE_BUNDLED_DEPENDENCIES option consistent with enchantum Jan 25, 2026
Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request aims to make the DREFLECTCPP_USE_BUNDLED_DEPENDENCIES option consistent for the enchantum dependency. The changes in CMakeLists.txt correctly add logic to find and link an external enchantum library when bundled dependencies are disabled. Correspondingly, the include paths for enchantum headers in various files have been updated to be more generic, allowing them to work with both the bundled and external versions. The implementation is correct and effectively addresses the issue described.

@liuzicheng1987
Copy link
Contributor

liuzicheng1987 commented Jan 25, 2026

@DNKpp , thanks for your PR!

However, the problem is that enchantum is not on Conan. If I accept this PR, it will for sure break our Conan package.

@liuzicheng1987
Copy link
Contributor

@DNKpp , it appears to me that the pragmatic solution is to have a separate flag for whether or not we want to used the bundled enchantum.

@DNKpp
Copy link
Author

DNKpp commented Jan 25, 2026

@liuzicheng1987 I think a separate option for each bundled dependency would be the best. I'm certain there are already lots of CTRE users out there, while yyjson is probably not that common in the c++ community. The all or nothing option is at least something, but I like your suggestion with dedicated switches.

EDIT: Are all these failed pipelines due to the mentioned conan issue?

@liuzicheng1987
Copy link
Contributor

@DNKpp , that means we would have to tweak the existing recipes. So I rather just have a separate flag for enchantum.

The difference between CTRE and YYJSON and enchantum is that the former two can be found on all relevant package managers. So if we do not include a bundled version, there is always an alternative. Enchantum, on the other hand , is not found on all relevant package managers, so we need to treat it differently.

@liuzicheng1987
Copy link
Contributor

@DNKpp , yes, all of the pipeline failures are related to the Conan issue.

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