Repository navigation
docs: update build - #335
Conversation
📝 WalkthroughWalkthroughThis PR renames pixi-invoked tasks in CI workflows, restructures Changes
Sequence Diagram(s)(No sequence diagram generated.) Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
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: 3
🧹 Nitpick comments (1)
pixi.toml (1)
107-112: Consider propagating build type to test task for consistency.The
testtask acceptstypeargument and passes it tounit-testandintegration-test, but doesn't include theciflag that was used during build. If the CI environment affects test behavior or paths, this could lead to inconsistencies.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pixi.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
.github/workflows/test-cmake.yml.github/workflows/test-xmake.ymlpixi.tomltests/conftest.py
🧰 Additional context used
🪛 Ruff (0.14.10)
tests/conftest.py
46-46: Undefined name path
(F821)
47-47: Undefined name path
(F821)
47-47: Undefined name path
(F821)
48-48: Undefined name path
(F821)
⏰ 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). (7)
- GitHub Check: xmake / build (ubuntu-24.04, debug)
- GitHub Check: cmake / build (macos-15, Debug)
- GitHub Check: xmake / build (ubuntu-24.04, releasedbg)
- GitHub Check: cmake / build (ubuntu-24.04, Debug)
- GitHub Check: xmake / build (macos-15, releasedbg)
- GitHub Check: xmake / build (windows-2025, releasedbg)
- GitHub Check: cmake / build (windows-2025, RelWithDebInfo)
🔇 Additional comments (5)
tests/conftest.py (1)
2-2: LGTM!The
sysimport is correctly added and is necessary for the Windows platform check below.pixi.toml (4)
17-22: Clean environment organization.The feature-based environment structure is well-organized, providing clear separation between build, development, packaging, formatting, and node environments. This improves maintainability over the previous flat structure.
163-173: Good use of inputs/outputs for incremental builds.The
install-docsandinstall-vscodetasks properly defineinputsandoutputsfor dependency caching, which enables pixi to skip redundantpnpm installruns when dependencies haven't changed.
186-197: Formatting tasks are comprehensive and well-structured.The format feature covers all relevant file types with appropriate tools. The exclusion of lock files (
package-lock.json,pnpm-lock.yaml) from formatting is a good practice.
96-105: Unix-style paths work cross-platform in this project.The test tasks already run on Windows as part of the CI pipeline (windows-2025 in test-cmake.yml), and no platform-specific path overrides are needed. Forward slashes are supported in modern shells and pixi handles them transparently.
Likely an incorrect or invalid review 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 (1)
editors/vscode/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (8)
docs/en/dev/build.mddocs/en/dev/extension.mddocs/zh/dev/build.mddocs/zh/dev/extension.mdeditors/vscode/.eslintrc.jsoneditors/vscode/package.jsoneditors/zed/src/clice.rspixi.toml
💤 Files with no reviewable changes (1)
- editors/vscode/.eslintrc.json
✅ Files skipped from review due to trivial changes (1)
- docs/en/dev/extension.md
🧰 Additional context used
🪛 LanguageTool
docs/en/dev/build.md
[grammar] ~3-~3: Ensure spelling is correct
Context: ...umes your local environment matches the prebuild environment closely (especially when en...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~75-~75: Ensure spelling is correct
Context: ... | ### XMake Build clice with: ```bash xmake f -c --mode=releas...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🪛 markdownlint-cli2 (0.18.1)
docs/zh/dev/build.md
7-7: Link fragments should be valid
(MD051, link-fragments)
⏰ 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). (8)
- GitHub Check: xmake / build (windows-2025, releasedbg)
- GitHub Check: cmake / build (windows-2025, RelWithDebInfo)
- GitHub Check: cmake / build (ubuntu-24.04, Debug)
- GitHub Check: xmake / build (ubuntu-24.04, releasedbg)
- GitHub Check: cmake / build (macos-15, Debug)
- GitHub Check: xmake / build (macos-15, releasedbg)
- GitHub Check: xmake / build (ubuntu-24.04, debug)
- GitHub Check: xmake / build (macos-15, debug)
🔇 Additional comments (9)
editors/zed/src/clice.rs (2)
5-7: LGTM: Simplified CliceBinary struct.The struct now only tracks the executable path, removing the resource_dir field. This simplification aligns with the removal of resource_dir handling throughout the extension.
10-41: This review comment is incorrect. The--resource-dirargument was never a valid CLI option for the clice binary. The clice binary initializes its resource directory automatically at startup from its executable path viafs::init_resource_dir(argv[0]). Theresource_dirconcept is internal to clice's compiler command building logic, not something LSP clients should pass as arguments. The current implementation passing only--modeandpipeis correct and consistent with the VSCode and Neovim extensions.Likely an incorrect or invalid review comment.
editors/vscode/package.json (2)
146-146: LGTM: Pretest script simplified.The removal of
pnpm run lintfrom the pretest script is consistent with the removal of ESLint configuration from the project. The script now only compiles before running tests.
151-163: Both dependency updates are compatible with the current codebase.The breaking changes in @types/node v25 and webpack-cli v6 do not impact this project:
- webpack v5.104.1 meets webpack-cli v6's minimum requirement (5.82.0)
- No deprecated webpack-cli commands (init, loader, plugin) are used
- No webpack-dev-server v4 dependency exists
- No deprecated Node.js APIs (SlowBuffer, etc.) are used
Minor note: Consider adding an explicit Node.js version constraint (e.g.,
.nvmrcor"engines"in package.json) since webpack-cli v6 requires Node.js ≥18.12.0, ensuring consistent developer environments.pixi.toml (3)
17-22: LGTM: Well-structured environment definitions.The environment blocks provide clear separation between build, test, package, format, and node workflows. This allows users to activate only the tools they need for their specific tasks.
67-112: LGTM: Well-structured cmake build and test tasks.The task definitions properly chain dependencies and propagate arguments through the build pipeline. The use of
depends-onwith parameterized tasks allows flexible configuration of build types.
157-177: LGTM: Efficient Node.js task definitions with caching.The install tasks properly utilize
inputsandoutputsfor build caching, and the build/publish tasks correctly depend on their respective install tasks. The use ofcwdensures commands run in the correct subdirectories.docs/zh/dev/extension.md (1)
1-70: All pixi tasks referenced in the documentation (install-vscode,build-vscode,publish-vscode) are properly defined in pixi.toml. No issues found.docs/zh/dev/build.md (1)
7-7: The link fragment is correct and will resolve properly.The heading
## 🛠️ Manual Buildgenerates the anchor IDmanual-buildin VitePress's default configuration. The markdown-it-anchor plugin (used by VitePress) strips emoji during slug generation, so the link reference#manual-buildat line 7 correctly resolves to the Manual Build section at line 47.
Summary by CodeRabbit
Bug Fixes
Chores
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.