-
Notifications
You must be signed in to change notification settings - Fork 67
[Intermediate]: Enable all build targets in CI on every PR #1277
Description
🧩 Intermediate Friendly
This issue is a good fit for contributors who are already familiar with the Hiero C++ SDK and feel comfortable navigating the codebase.
Intermediate Issues often involve:
- Exploring existing implementations
- Understanding how different components work together
- Making thoughtful changes that follow established patterns
The goal is to support deeper problem-solving while keeping the task clear, focused, and enjoyable to work on.
Important
🧭 About Intermediate Issues
Intermediate Issues are a great next step for contributors who enjoy digging into the codebase and reasoning about how things work.
These issues often:
- Involve multiple related files or components
- Encourage investigation and understanding of existing behavior
- Leave room for thoughtful implementation choices
- Stay focused on a clearly defined goal
🐞 Problem Description
The CI build for PRs does not enable all available build targets. Two targets are currently missing: the TCK server binary and the example programs. This means type errors and build failures in src/tck/src/ and src/sdk/examples/ can be merged undetected.
The project defines four build flags (documented in README.md):
| Flag | What it builds |
|---|---|
BUILD_TESTS=ON |
SDK unit and integration tests |
BUILD_TCK=ON |
TCK server binary (src/tck/src/) |
BUILD_TCK_TESTS=ON |
TCK server unit tests |
BUILD_EXAMPLES=ON |
~66 standalone example programs (src/sdk/examples/) |
The primary build job in .github/workflows/zxc-build-library.yaml currently configures CMake with only two of the four:
cmake --preset linux-x64-debug -DBUILD_TESTS=ON -DBUILD_TCK_TESTS=ON
-DBUILD_TCK=ON and -DBUILD_EXAMPLES=ON are both absent. As a result, the compiler never sees src/tck/src/ or src/sdk/examples/ on PRs.
This gap was confirmed when issue #1272 was filed: src/tck/src/file/FileService.cc contained pointer-style dereferences (->) on a value-type field (KeyList mAdminKeys), which are hard compile errors. The code had been merged via PR #1252 because the TCK server binary was never built in CI, so the compiler never saw the error.
Relevant files:
.github/workflows/zxc-build-library.yaml— the reusable build workflow called on every PR.github/workflows/flow-pull-request-checks.yaml— the PR-triggered workflow that callszxc-build-library.yamlREADME.md— lists all available build flags
💡 Expected Outcome
Every PR build should compile all four targets so that type errors and broken code in any part of the repository are caught before merge.
The CMake configure step in both the debug and release builds should enable all flags:
cmake --preset linux-x64-debug -DBUILD_TESTS=ON -DBUILD_TCK=ON -DBUILD_TCK_TESTS=ON -DBUILD_EXAMPLES=ON
The change should:
- Add
-DBUILD_TCK=ONand-DBUILD_EXAMPLES=ONto the CMake configure invocations in.github/workflows/zxc-build-library.yaml - Apply to both the debug build (runs on every PR) and the release build (runs on merge)
- Not remove or weaken any existing flags
- Not change the test run steps
🧠 Implementation Notes
Suggested approach:
-
Open
.github/workflows/zxc-build-library.yamland locate the CMake Build (Debug) step (around line 156). -
Update the configure line to include all four flags:
${{ steps.cgroup.outputs.exec }} cmake --preset ${{ matrix.preset }}-debug \ -DBUILD_TESTS=ON -DBUILD_TCK=ON -DBUILD_TCK_TESTS=ON -DBUILD_EXAMPLES=ON
-
Repeat the same change for the CMake Build (Release) step (around line 168).
-
Check
README.mdto confirm the full list of available flags and verify nothing else is obviously missing. -
Before opening the PR, do a local build with all four flags enabled and check for any compilation errors in
src/tck/src/orsrc/sdk/examples/. If you find build failures, please either open a separate issue for each one or tag a maintainer in your PR so they can create the tracking issues. Do not include unrelated build fixes in this PR — keep the scope to the workflow change only.
There are no C++ source changes in this issue — the work is entirely in the workflow YAML file.
✅ Acceptance Criteria
To help get this change merged smoothly:
-
-DBUILD_TCK=ONand-DBUILD_EXAMPLES=ONare present in the CMake configure step for the debug build inzxc-build-library.yaml -
-DBUILD_TCK=ONand-DBUILD_EXAMPLES=ONare present in the CMake configure step for the release build inzxc-build-library.yaml - No existing flags are removed or weakened
- All existing CI checks pass on the PR
- The PR description uses
Fixes #<issue-number>so the issue closes on merge
📋 Contribution Guide
To help your contribution go as smoothly as possible, we recommend following these steps:
- Comment
/assignto request the issue - Wait for assignment
- Fork the repository and create a branch
- Set up the project using the instructions in
README.md - Make the requested changes
- Sign each commit using
-s -S - Push your branch and open a pull request
Read Workflow Guide for step-by-step workflow guidance. Read README.md for setup instructions.
❗ Pull requests cannot be merged without S and s signed commits. See the Signing Guide.
📚 Additional Context or Resources
This gap was surfaced by issue #1272, which reported a build failure in src/tck/src/file/FileService.cc — a type error that had been merged because the TCK server was never compiled in CI.
Related items:
- Issue [Beginner]: Build failure in TCK FileService.cc due to FileInfo mAdminKeys API change #1272 — the build failure that exposed this CI gap
- PR feat: implemented rest of the TCK endpt for FileService. #1252 — the PR that introduced the broken TCK code (merged without CI catching it)
If you have questions, the community is happy to help:
https://discord.com/channels/905194001349627914/1337424839761465364