coderabbitai[bot]
commented
2 weeks ago Walkthrough
The pull request introduces updates to the GitHub Actions workflows and documentation related to macOS builds. The workflow for building on macOS has been modified to support a wider range of macOS versions by implementing a matrix strategy and updating installation paths. Additionally, documentation has been revised to generalize macOS setup instructions, removing specific references to macOS 12. Changes include updates to exclusion patterns in workflow triggers and adjustments in the installation script paths across various documentation files.
Changes
| File Path | Change Summary |
|---|---|
| .github/workflows/clp-core-build-macos.yaml | Updated runs-on directive to use a matrix for "macos-13" and "macos-14"; modified installation script path; adjusted paths monitored for changes. |
| .github/workflows/clp-core-build.yaml | Changed exclusion pattern for macOS installation scripts in workflow triggers to include all macOS versions. |
| docs/src/dev-guide/components-core/index.md | Updated references from "macOS 12" to "macOS" in requirements and setup instructions; modified table of contents. |
| docs/src/dev-guide/components-core/macos-deps-install.md | Changed header from "macOS 12 setup" to "macOS setup"; updated installation script path to reflect broader applicability. |
Possibly related PRs
-
552: This PR modifies the same workflow file (
.github/workflows/clp-core-build-macos.yaml) as the main PR, specifically addressing the build process on macOS by adding a command to remove conflicting Go binaries, which is relevant to the overall macOS build configuration changes made in the main PR.
📜 Recent review details
**Configuration used: CodeRabbit UI** **Review profile: CHILL**📥 Commits
Files that changed from the base of the PR and between 829013bc94d87baf2a30ff28a5495de469a1d389 and 1a8f64eb3b83c0e4080b27a8046fb3c8079001e1.📒 Files selected for processing (4)
* .github/workflows/clp-core-build-macos.yaml (4 hunks) * .github/workflows/clp-core-build.yaml (2 hunks) * docs/src/dev-guide/components-core/index.md (2 hunks) * docs/src/dev-guide/components-core/macos-deps-install.md (2 hunks)✅ Files skipped from review due to trivial changes (2)
* .github/workflows/clp-core-build.yaml * docs/src/dev-guide/components-core/macos-deps-install.md🔇 Additional comments (6)
.github/workflows/clp-core-build-macos.yaml (4)
`11-11`: **Path pattern update looks good, eh!** The generalized path pattern for macOS installation scripts aligns well with the goal of supporting multiple macOS versions. --- `24-24`: **Same path pattern update as above** --- `59-59`: **Installation script path update looks good, but let's verify the script location.** The generalized path aligns with the version-agnostic approach. However, we should verify that the installation script exists at the new location and maintains compatibility across macOS versions. ```shell #!/bin/bash # Description: Verify installation script existence and contents # Check if the script exists echo "Checking for installation script..." if [ -f "./components/core/tools/scripts/lib_install/macos/install-all.sh" ]; then echo "Script found. Checking for version-specific commands..." # Look for any remaining version-specific references rg -i "macos-1[234]" "./components/core/tools/scripts/lib_install/macos/install-all.sh" else echo "Warning: Installation script not found at expected location" fi ``` --- `41-43`: **Matrix strategy implementation looks solid, but let's verify architecture coverage.** The matrix configuration with macos-13 and macos-14 runners is well-structured. However, we should verify that this covers both x86 and ARM architectures as intended.✅ Verification successful
**Architecture coverage is adequately handled by the matrix configuration** Based on the search results, we can confirm that: - The codebase primarily uses architecture-agnostic code - Build dependencies are properly configured for x86 through the dependency images - No explicit ARM-specific code or configurations that would require special handling - macOS-13 (Intel) and macOS-14 (Apple Silicon) runners will naturally build for their respective architectures The matrix strategy using both macos-13 and macos-14 will effectively cover both x86 and ARM architectures without requiring additional configuration.🏁 Scripts executed
The following scripts were executed for the analysis: Script: ```shell #!/bin/bash # Description: Check if there are any architecture-specific build configurations or tests # that might need adjustment for the new runners # Look for architecture-specific configurations echo "Checking for architecture-specific configurations..." rg -i "arch|x86|arm|aarch64" ./components/core/cmake/ rg -i "arch|x86|arm|aarch64" ./components/core/CMakeLists.txt # Check for potential architecture-dependent code echo "Checking for architecture-dependent code..." rg -i "arch|x86|arm|aarch64" ./components/core/src/ ``` Length of output: 438 --- Script: ```shell #!/bin/bash # Let's try a different approach to check for architecture-specific configurations # Check workflow files for architecture-specific settings echo "Checking workflow files for architecture configurations..." rg -i "arch|x86|arm|aarch64" .github/workflows/ # Check build scripts and installation files echo "Checking build and installation scripts..." fd -e sh -e bash . ./components/core/tools/scripts/ -x rg -i "arch|x86|arm|aarch64" {} # Check CMake files with different path approach echo "Checking CMake files..." fd CMakeLists.txt . -x rg -i "arch|x86|arm|aarch64" {} fd -e cmake . -x rg -i "arch|x86|arm|aarch64" {} ``` Length of output: 15371docs/src/dev-guide/components-core/index.md (2)
`57-57`: **LGTM! The generalized macOS reference improves maintainability.** The removal of the specific macOS version from the documentation link text aligns well with the PR's objective to support multiple macOS versions. --- `101-101`: **LGTM! Consistent update to the toctree entry.** The toctree entry has been appropriately updated to match the generalized macOS documentation structure.Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?
kirkrodrigues
Description
The GH macos-12 hosted runner image has begun to be deprecated leading to much longer build times for our macOS workflow.
This PR addresses the issue by:
core.Validation performed
task docs:serveand validated the docs about installing on macOS were updated.Summary by CodeRabbit
New Features
Documentation