voyager icon indicating copy to clipboard operation
voyager copied to clipboard

C++ improvements

Open markkohdev opened this issue 11 months ago • 3 comments

Description

In this PR we will:

  • Clean up the C++ module structure
  • Add CMake for standalone C++ compilation
  • Improve formatting within the c++ module
  • Add doctest testing framework for C++ unit tests
    • Note: Right now we have a single test. In order to properly utilize DocTest we need to first provide namespacing for this library as per #52 -- otherwise we run into issues with ambiguous function calls during doctest stringification.
  • Update documentation to be more comprehensive around building, testing, etc.
  • Add a github pull request template :)

Changes Made

C++

Main changes:

  • Break cpp dir into src and test directories
  • Add cmake and documentation around it
  • Add an initial test
  • Run clang-format with new 120 line length standard

Python

  • Remove c++ formatting of python bindings from tox
  • Remove voyager-headers link since it doesn't seem to be needed
  • Update location of C++ files

Java

  • Update location of C++ files
  • Run formatter
  • Update docs

Testing

Checklist

  • [x] My code follows the code style of this project.
  • [x] I have added and/or updated appropriate documentation (if applicable).
  • [x] All new and existing tests pass locally with these changes.
  • [x] I have run static code analysis (if available) and resolved any issues.
  • [x] I have considered backward compatibility (if applicable).
  • [x] I have confirmed that this PR does not introduce any security vulnerabilities.

Additional Comments

markkohdev avatar Mar 22 '24 23:03 markkohdev

without any pressure - is there a timeframe for merging this? just asking for our own planning on when to start integrating these changes

alexander-riss avatar Apr 15 '24 08:04 alexander-riss

just a friendly ping - any update on this? do you think it makes sense for potential users to start picking up your branch directly instead of waiting for this to be merged? @markkohdev

alexander-riss avatar May 13 '24 09:05 alexander-riss

This should help us get closer to solving #11 and #3 (although we may address this first in the Java bindings)

markkohdev avatar Jul 01 '24 14:07 markkohdev