build: optimize the workflow - #331
Conversation
📝 WalkthroughWalkthroughCentralizes CI into a main orchestrator that path-filters changes and conditionally invokes reusable workflows, migrates many workflows to Pixi-driven tasks (workflow_call), adds docs deployment to GitHub Pages, updates pixi.toml for Node tasks and PNPM, and standardizes macOS naming across docs, headers, and tests. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (5)
docs/package-lock.jsonis excluded by!**/package-lock.jsondocs/pnpm-lock.yamlis excluded by!**/pnpm-lock.yamleditors/vscode/package-lock.jsonis excluded by!**/package-lock.jsoneditors/vscode/pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlpixi.lockis excluded by!**/*.lock
📒 Files selected for processing (2)
.github/workflows/deploy-docs.ymlpixi.toml
🔇 Additional comments (8)
pixi.toml (4)
62-62: LGTM!The exclusion of
pnpm-lock.yamlis appropriate since pnpm is being introduced as the package manager.
76-78: LGTM!The Node.js and pnpm dependencies are correctly specified to support the documentation build workflow.
84-94: LGTM!The install tasks correctly use pnpm and implement proper dependency tracking with inputs/outputs for caching optimization.
106-106: LGTM!The node environment is correctly defined to include the node feature.
.github/workflows/deploy-docs.yml (4)
10-14: Good addition!Adding the pull_request trigger enables validation of documentation changes before merging, which improves quality control.
27-34: LGTM!The Pixi setup is correctly configured with the node environment, caching enabled, and locked mode for reproducible builds.
37-37: LGTM!The build-docs command correctly invokes the Pixi task defined in pixi.toml.
40-40: LGTM!The conditional deployment ensures that only changes merged to main are deployed to GitHub Pages, while PRs only validate the build.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
.github/workflows/deploy-docs.yml (1)
8-8: Fix workflow path references.The workflow references
.github/workflows/deploy.ymlbut the actual filename isdeploy-docs.yml. This mismatch means the workflow won't trigger when the workflow file itself is modified.🔎 Proposed fix
- - ".github/workflows/deploy.yml" + - ".github/workflows/deploy-docs.yml"Apply this fix at both lines 8 and 14.
Also applies to: 14-14
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
editors/vscode/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (4)
.github/workflows/deploy-docs.yml.github/workflows/publish-vscode.ymleditors/vscode/package.jsonpixi.toml
🚧 Files skipped from review as they are similar to previous changes (1)
- pixi.toml
🔇 Additional comments (8)
.github/workflows/publish-vscode.yml (2)
1-1: LGTM!The workflow name and formatting improvements are appropriate and improve readability.
Also applies to: 8-8, 14-16
42-45: Thebuild-vscodeandpublish-vscodetasks are properly defined inpixi.tomlwith the correct working directory (editors/vscode) and dependencies. No issues found..github/workflows/deploy-docs.yml (4)
10-15: LGTM!Adding the pull_request trigger is a good practice to validate docs builds before merging.
40-40: LGTM!The conditional deployment ensures that only builds from the main branch are deployed to GitHub Pages, which is a good practice to prevent PRs from publishing unreviewed documentation.
37-37: Thebuild-docstask is correctly defined inpixi.toml.The task is properly configured at line 81 with the working directory set to
docs, commandpnpm run docs:build, and a dependency oninstall-docs. No issues detected.
27-34: The versions are already current. setup-pixi v0.9.3 (released Nov 3, 2025) and pixi v0.61.0 (released Dec 9, 2025) are the latest stable versions. Thebuild-docstask is properly defined in pixi.toml as a dependency of the node feature.Likely an incorrect or invalid review comment.
editors/vscode/package.json (2)
144-146: LGTM!The migration from npm to pnpm is consistent across all scripts. The updated
baseImagesUrlcorrectly points to the main branch, and the--no-dependenciesflag aligns with the new pnpm-based workflow.Also applies to: 149-150
175-180: LGTM!The pnpm configuration appropriately restricts built dependencies to
@vscode/vsce-signandkeytar, which are known native dependencies requiring compilation. This helps optimize installation time and reduces potential build issues.
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
.github/workflows/build-llvm.yml.github/workflows/check-format.yml.github/workflows/deploy-docs.yml.github/workflows/main.yml.github/workflows/publish-clice.yml.github/workflows/publish-vscode.yml.github/workflows/test-cmake.yml.github/workflows/test-xmake.ymldocs/en/dev/build.mddocs/zh/dev/build.mdinclude/Test/Platform.htests/unit/Compiler/CommandTests.cpp
💤 Files with no reviewable changes (1)
- .github/workflows/build-llvm.yml
✅ Files skipped from review due to trivial changes (1)
- docs/en/dev/build.md
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/publish-vscode.yml
🧰 Additional context used
🪛 actionlint (1.7.9)
.github/workflows/deploy-docs.yml
29-29: the runner of "peaceiris/actions-gh-pages@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
.github/workflows/main.yml
59-59: could not read reusable workflow file for "./.github/workflows/format.yml": open /home/jailuser/git/.github/workflows/format.yml: no such file or directory
(workflow-call)
🔇 Additional comments (14)
docs/zh/dev/build.md (1)
7-7: LGTM!The documentation updates correctly standardize the platform name to "macOS" in both the platform list and the LLVM libs notes, aligning with Apple's official capitalization.
Also applies to: 69-69
tests/unit/Compiler/CommandTests.cpp (1)
237-237: LGTM!The test gating conditions correctly updated to use
macOSinstead ofMacOS, matching the constant rename inPlatform.h. These changes maintain consistency even though the test cases are currently commented out.Also applies to: 274-274
include/Test/Platform.h (1)
12-14: Renaming fromMacOStomacOSis complete—no breaking changes remain unaddressed.All references to the old constant name have been updated. The only remaining "MacOS" reference in the codebase is a comment about "MacOSX" in
src/Compiler/Toolchain.cpp, which does not affect the constant definition. The standardized naming aligns with Apple's official capitalization and is correctly used in test code..github/workflows/check-format.yml (1)
3-4: LGTM! Workflow correctly converted to reusable pattern.The conversion from direct triggers to
workflow_callaligns with the new orchestration model introduced inmain.yml, enabling conditional invocation based on path filters..github/workflows/test-cmake.yml (2)
3-4: LGTM! Workflow correctly converted to reusable pattern.The conversion to
workflow_callenables invocation from the central orchestrator workflow.
32-36: LGTM! Command simplification improves maintainability.The consolidated single-line Pixi commands are cleaner and align with the project's standardization on Pixi-based task execution.
.github/workflows/publish-clice.yml (3)
1-1: LGTM! Workflow rename improves clarity.The rename from "package" to "clice" better reflects the workflow's purpose.
6-10: LGTM! Workflow correctly converted to reusable pattern.The conversion to
workflow_calland job renaming align with the orchestration model inmain.yml.
69-87: The upload behavior is correct. Whenpublish-clice.ymlis called viaworkflow_callfrommain.yml, the event context is inherited as expected:
- Tag-based releases (
release-clice): triggered by push events with tags matchingv*, sogithub.event_name == 'push'evaluates to true and uploads execute- PR-based checks (
call-clice-check): triggered by pull_request events, sogithub.event_name == 'push'evaluates to false and uploads are correctly skippedThe condition is working as intended.
.github/workflows/test-xmake.yml (2)
3-4: LGTM! Workflow correctly converted to reusable pattern.Consistent with the conversion pattern applied across other test workflows.
41-45: LGTM! Command simplification improves consistency.The single-line Pixi commands align with the standardization effort across all workflows.
.github/workflows/deploy-docs.yml (1)
1-26: LGTM! Docs deployment workflow is well-structured.The workflow correctly uses
workflow_callfor orchestration, sets up the Node.js environment via Pixi, and builds docs using the project's standardized task system..github/workflows/main.yml (2)
1-55: LGTM! Well-designed orchestration workflow.The central orchestrator with path-based filtering is an excellent approach to conditionally dispatch reusable workflows, reducing redundant CI runs while maintaining comprehensive coverage.
88-96: LGTM! Release workflow dispatch is correctly configured.Tag-based releases properly invoke both clice and vscode publish workflows with inherited secrets.
Summary by CodeRabbit
New Features
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.