Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the Reelay project to version 25.12.0 (from 23.8), introducing modernized build configurations, updated dependencies, and improved CI/CD infrastructure. Key changes include migration from manual dependency management to CMake FetchContent, containerization restructuring from Docker to Podman/Buildah, and comprehensive CI workflow updates.
- Version bump from 23.8 to 25.12.0 with addition of PATCH version component
- Build system modernization: CMake minimum version raised to 3.26, dependencies now fetched via FetchContent
- CI/CD overhaul: new container build workflow, Python testing matrix expanded, benchmark automation added
- Container infrastructure relocated from
docker/tocontainers/directory with Buildah-based multi-architecture builds
Reviewed changes
Copilot reviewed 28 out of 30 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| CMakeLists.txt | Major refactoring: FetchContent for Boost/CUDD dependencies, renamed build options with REELAY_ prefix, version parsing updated for 3-component versioning |
| include/reelay/version.hpp | Version updated to 25.12.0, added PATCH component |
| python/reelay/init.py | Python package version updated to match C++ library version |
| pyproject.toml | Updated CMake minimum version to 3.26, renamed build options, added pytest as dev dependency |
| tests/CMakeLists.txt | Catch2 version constraint added (3.4), formatting improvements |
| src/CMakeLists.txt | Build option renamed from BUILD_PYTHON_BINDINGS to REELAY_BUILD_PYTHON_BINDINGS |
| src/pybind11/CMakeLists.txt | Migrated from find_package to FetchContent for pybind11 dependency |
| apps/ryjson1/CMakeLists.txt | Migrated from find_package to FetchContent for simdjson dependency |
| include/reelay/unordered_data.hpp | Updated CUDD include path from separate headers to unified cudd/cudd.hpp |
| include/reelay/intervals.hpp | Updated CUDD include path to cudd/cudd.hpp |
| .github/workflows/tests.yml | Removed container dependency, added inline dependency installation |
| .github/workflows/python.yml | Expanded Python version matrix (3.9-3.14), updated actions versions, removed container dependency |
| .github/workflows/containers.yml | New multi-architecture container build workflow using Buildah |
| .github/workflows/wheels.yml | New workflow for building Python wheels with cibuildwheel |
| .github/workflows/benchmark.yml | New workflow for automated performance benchmarking using hyperfine |
| .github/workflows/cryjson.yml | Removed obsolete RyJSON container workflow |
| .github/workflows/cdevel.yml | Removed obsolete development container workflow |
| containers/devel/Dockerfile | New development container definition based on Ubuntu 24.04 |
| containers/ryjson/Dockerfile | New runtime container definition for ryjson application |
| containers/release/Dockerfile | New release container based on manylinux_2_28 |
| containers/scripts/boost-source-cmake-install.sh | New script for building Boost from source with CMake |
| benchmarks/ryjson-benchmark.sh | New benchmark script using hyperfine for temporal logic patterns |
| Makefile | Added new targets (apps, clean), reorganized existing targets |
| .devcontainer/devcontainer.json | Updated to reference new container location |
| .gitignore | Added .venv/ and benchmarks/data to ignore list |
| docker/ryjson/Dockerfile | Removed legacy Docker file |
| docker/devel/Dockerfile | Removed legacy Docker file |
| .devcontainer/Dockerfile | Removed legacy devcontainer Dockerfile |
| cmake/reelayConfig.cmake.in | New CMake package configuration template |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| - name: Prepare environment variables | ||
| run: | | ||
| echo "REELAY_VERSION=$(git rev-parse HEAD)" >> $GITHUB_ENV | ||
|
|
There was a problem hiding this comment.
The variable $repo is undefined in this script. It should be set to ${{ github.repository }} or the appropriate repository identifier before being used in the curl commands.
| repo="$GITHUB_REPOSITORY" |
| echo "REELAY_VERSION=$(git rev-parse HEAD)" >> $GITHUB_ENV | ||
|
|
||
| tag=$(curl -s "https://api.github.com/repos/$repo/releases/latest" | jq -r '.tag_name') | ||
| ref_info=$(curl -s "https://api.github.com/repos/$repo/git/ref/tags/$tag") |
There was a problem hiding this comment.
The variable $repo is undefined in this script. It should be set to ${{ github.repository }} or the appropriate repository identifier before being used in the curl commands.
| if [ "$obj_type" = "commit" ]; then | ||
| commit_sha="$tag_sha" | ||
| else | ||
| commit_sha=$(curl -s "https://api.github.com/repos/$repo/git/tags/$tag_sha" | jq -r '.object.sha') |
There was a problem hiding this comment.
The variable $repo is undefined in this script. It should be set to ${{ github.repository }} or the appropriate repository identifier before being used in the curl commands.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 32 out of 35 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| DOWNLOAD_EXTRACT_TIMESTAMP | ||
| TRUE | ||
| FIND_PACKAGE_ARGS | ||
| 1.82.0 |
There was a problem hiding this comment.
Boost version mismatch: The download URL specifies boost-1.89.0 but FIND_PACKAGE_ARGS specifies version 1.82.0. This inconsistency could lead to confusion. The FIND_PACKAGE_ARGS version should be updated to match the downloaded version (1.89.0).
| 1.82.0 | |
| 1.89.0 |
| std::string yname = "value"; | ||
| }; | ||
|
|
||
| static std::array<struct argp_option, 12> options = { |
There was a problem hiding this comment.
Incorrect array size: The options array is declared with size 12, but only contains 11 elements (10 options plus the nullptr terminator). This could lead to uninitialized memory access. Either change the size to 11 or remove the explicit size declaration to let the compiler deduce it.
| static std::array<struct argp_option, 12> options = { | |
| static std::array<struct argp_option, 11> options = { |
| using input_t = simdjson::dom::element; | ||
| using output_t = reelay::json; |
There was a problem hiding this comment.
Duplicate type alias declarations: input_t and output_t are declared twice (lines 225-226 and lines 252-253). The first declarations on lines 225-226 should be removed to avoid code duplication.
No description provided.