-
Notifications
You must be signed in to change notification settings - Fork 2
Build examples in cicd #12
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughA new GitHub Actions workflow named "Examples" was added to automate building and testing example projects on Linux, Windows, and macOS. Additionally, the existing "tests" workflow was updated to use Changes
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
🧰 Additional context used🧠 Learnings (1)📓 Common learnings
🪛 YAMLlint (1.37.1).github/workflows/examples.yml[error] 14-14: trailing spaces (trailing-spaces) [warning] 21-21: wrong indentation: expected 8 but found 10 (indentation) [warning] 30-30: wrong indentation: expected 6 but found 8 (indentation) ⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (4)
.github/workflows/examples.yml (4)
12-14
: Remove trailing whitespace to keep YAML-lint greenLine 14 contains nothing but spaces, triggering
trailing-spaces
errors.
Delete the line or strip the spaces:- +
18-22
: Fix list formatting – mis-indented closing bracket
YAMLlint
flags the]
on line 21 (expected 8 but found 10
).
Either re-indent or switch to block-style lists for better readability:- image: [ - "test_ubuntu:2404" - ] + image: + - test_ubuntu:2404
28-31
: Aligncontainer.image
indentationLine 30 is indented two spaces too far (
indentation
warning). Adjust to match surrounding keys:- container: - image: ghcr.io/cmakelib/${{ matrix.image }} + container: + image: ghcr.io/cmakelib/${{ matrix.image }}
61-66
: Use canonical PowerShell syntax for env vars and pathsPrefer
$Env:
(capital E) andJoin-Path
to avoid quoting/path issues:- git clone --branch "v$env:CMLIB_VERSION" https://github.com/cmakelib/cmakelib.git - $env:CMLIB_DIR = "$(pwd)/cmakelib" + git clone --branch "v$Env:CMLIB_VERSION" https://github.com/cmakelib/cmakelib.git + $Env:CMLIB_DIR = Join-Path -Path (Get-Location) -ChildPath 'cmakelib'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/examples.yml
(1 hunks).github/workflows/tests.yml
(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: koudis
PR: cmakelib/cmakelib-component-cmdef#11
File: test/CMakeLists.txt:9-11
Timestamp: 2025-06-17T22:15:13.853Z
Learning: The cmakelib-component-cmdef project uses a custom testing framework with TEST.cmake and custom macros like TEST_RUN, TEST_RUN_AND_CHECK_OUTPUT rather than CTest. CTest integration is not needed or used in this project.
.github/workflows/tests.yml (1)
Learnt from: koudis
PR: cmakelib/cmakelib-component-cmdef#11
File: test/CMakeLists.txt:9-11
Timestamp: 2025-06-17T22:15:13.853Z
Learning: The cmakelib-component-cmdef project uses a custom testing framework with TEST.cmake and custom macros like TEST_RUN, TEST_RUN_AND_CHECK_OUTPUT rather than CTest. CTest integration is not needed or used in this project.
🪛 YAMLlint (1.37.1)
.github/workflows/examples.yml
[error] 14-14: trailing spaces
(trailing-spaces)
[warning] 21-21: wrong indentation: expected 8 but found 10
(indentation)
[warning] 30-30: wrong indentation: expected 6 but found 8
(indentation)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: test_windows
- GitHub Check: test_linux (test_fedora:41)
- GitHub Check: test_macos
🔇 Additional comments (1)
.github/workflows/tests.yml (1)
12-14
: Confirmv1.1.0
tag exists before mergingThe workflow now pins
CMLIB_VERSION
to1.1.0
. If the tagv1.1.0
has not yet been published in thecmakelib
repository, everygit clone --branch "v${CMLIB_VERSION}" …
step will fail and break CI.
Double-check that the tag is live on GitHub, or postpone the merge until it is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.github/workflows/examples.yml (1)
61-66
: Use PowerShell-native path handling forCMLIB_DIR
Mixing Unix slashes with PowerShell’s
$env:
may work but is brittle (e.g., drives other thanC:
). PreferJoin-Path
to construct the path.- git clone --branch "v$env:CMLIB_VERSION" https://github.com/cmakelib/cmakelib.git - $env:CMLIB_DIR = "$(pwd)/cmakelib" + git clone --branch "v$env:CMLIB_VERSION" https://github.com/cmakelib/cmakelib.git + $env:CMLIB_DIR = Join-Path -Path (Get-Location) -ChildPath 'cmakelib'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/examples.yml
(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: koudis
PR: cmakelib/cmakelib-component-cmdef#11
File: test/CMakeLists.txt:9-11
Timestamp: 2025-06-17T22:15:13.853Z
Learning: The cmakelib-component-cmdef project uses a custom testing framework with TEST.cmake and custom macros like TEST_RUN, TEST_RUN_AND_CHECK_OUTPUT rather than CTest. CTest integration is not needed or used in this project.
🪛 YAMLlint (1.37.1)
.github/workflows/examples.yml
[error] 14-14: trailing spaces
(trailing-spaces)
[warning] 21-21: wrong indentation: expected 8 but found 10
(indentation)
[warning] 30-30: wrong indentation: expected 6 but found 8
(indentation)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test_windows
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
example/library/CMakeLists.txt (1)
15-17
: Same global flag scope caution as noted inlibrary-interface
Applying
CMAKE_WINDOWS_EXPORT_ALL_SYMBOLS
globally may unintentionally export symbols from future libraries. Consider the per-target approach outlined in the previous comment.
🧹 Nitpick comments (1)
example/library-interface/CMakeLists.txt (1)
16-18
: Prefer per-targetWINDOWS_EXPORT_ALL_SYMBOLS
instead of a global switchTurning the cache variable on makes every shared library created after this point auto-export its symbols—including any 3rd-party libs pulled in later. If the goal is to affect only
testlib-shared
, set the property on that target after it is declared:-IF(CMDEF_OS_WINDOWS) - SET(CMAKE_WINDOWS_EXPORT_ALL_SYMBOLS ON) -ENDIF() +IF(CMDEF_OS_WINDOWS) + # After add_library(testlib …) + SET_PROPERTY(TARGET testlib-shared PROPERTY WINDOWS_EXPORT_ALL_SYMBOLS ON) +ENDIF()This keeps symbol visibility predictable and avoids accidental leaks from unrelated libraries.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
example/library-interface/CMakeLists.txt
(1 hunks)example/library/CMakeLists.txt
(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: koudis
PR: cmakelib/cmakelib-component-cmdef#11
File: test/CMakeLists.txt:9-11
Timestamp: 2025-06-17T22:15:13.853Z
Learning: The cmakelib-component-cmdef project uses a custom testing framework with TEST.cmake and custom macros like TEST_RUN, TEST_RUN_AND_CHECK_OUTPUT rather than CTest. CTest integration is not needed or used in this project.
example/library-interface/CMakeLists.txt (2)
Learnt from: koudis
PR: cmakelib/cmakelib-component-cmdef#11
File: test/CMakeLists.txt:9-11
Timestamp: 2025-06-17T22:15:13.853Z
Learning: The cmakelib-component-cmdef project uses a custom testing framework with TEST.cmake and custom macros like TEST_RUN, TEST_RUN_AND_CHECK_OUTPUT rather than CTest. CTest integration is not needed or used in this project.
Learnt from: koudis
PR: cmakelib/cmakelib-component-cmdef#11
File: test/CMDEF_INSTALL/test_cases/install_related/include_dirs/CMakeLists.txt:0-0
Timestamp: 2025-06-16T19:47:01.662Z
Learning: In the CMDEF_INSTALL system, export destination paths intentionally contain double slashes like `${CMDEF_LIBRARY_INSTALL_DIR}/cmake//` - this is the actual format produced by the implementation, not a formatting error in tests.
example/library/CMakeLists.txt (2)
Learnt from: koudis
PR: cmakelib/cmakelib-component-cmdef#11
File: test/CMDEF_INSTALL/test_cases/install_related/include_dirs/CMakeLists.txt:0-0
Timestamp: 2025-06-16T19:47:01.662Z
Learning: In the CMDEF_INSTALL system, export destination paths intentionally contain double slashes like `${CMDEF_LIBRARY_INSTALL_DIR}/cmake//` - this is the actual format produced by the implementation, not a formatting error in tests.
Learnt from: koudis
PR: cmakelib/cmakelib-component-cmdef#11
File: test/CMakeLists.txt:9-11
Timestamp: 2025-06-17T22:15:13.853Z
Learning: The cmakelib-component-cmdef project uses a custom testing framework with TEST.cmake and custom macros like TEST_RUN, TEST_RUN_AND_CHECK_OUTPUT rather than CTest. CTest integration is not needed or used in this project.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: test_windows
- GitHub Check: test_macos
b8eaeb5
to
357135f
Compare
Summary by CodeRabbit