chore: use pixi to manage the format tools and format the world#330
Conversation
📝 WalkthroughWalkthroughCMake and xmake build targets were updated to use new source paths for two executables; CI/formatting workflows and tool configs were added or modified; many docs and editor files received formatting, front-matter, or minor content edits. Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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: 1
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
CMakeLists.txtsrc/clice.cctests/unit/unit_tests.cc
⏰ 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: build (macos-15, Debug)
- GitHub Check: build (windows-2025, RelWithDebInfo)
- GitHub Check: build (ubuntu-24.04, Debug)
- GitHub Check: build (macos-15, debug)
- GitHub Check: build (ubuntu-24.04, releasedbg)
- GitHub Check: build (ubuntu-24.04, debug)
- GitHub Check: build (macos-15, releasedbg)
- GitHub Check: build (windows-2025, releasedbg)
🔇 Additional comments (1)
CMakeLists.txt (1)
199-199: File move verified. The test source file has been successfully moved frombin/unit_tests.cctotests/unit/unit_tests.cc, and no stray references to the old path remain in the codebase.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
docs/clice.toml (1)
19-23: Consider adding a comment forlogging_dirfor consistency.Other directory settings (
cache_dir,index_dir) have descriptive comments, butlogging_diris missing one.🔎 Suggested improvement
# Directory for storing index files. index_dir = "${workspace}/.clice/index" +# Directory for storing log files. logging_dir = "${workspace}/.clice/logging"pixi.toml (1)
76-79: Hardcoded build path in test tasks.The
unit-testandintegration-testtasks assume./build/bin/as the binary location. This is reasonable for CI but may not match local development setups using different build directories.Consider documenting this assumption or making the path configurable if developers commonly use different build directories.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pixi.lockis excluded by!**/*.lock
📒 Files selected for processing (43)
.clang-format.github/ISSUE_TEMPLATE/bug_report.md.github/ISSUE_TEMPLATE/feature_request.md.github/workflows/check-format.yml.prettierrc.yamlREADME.mdconfig/llvm-manifest.jsondocs/.vitepress/config.tsdocs/.vitepress/theme/index.tsdocs/.vitepress/theme/sidebar.tsdocs/.vitepress/theme/style.cssdocs/clice.tomldocs/en/dev/contribution.mddocs/en/dev/test-and-debug.mddocs/en/guide/configuration.mddocs/en/index.mddocs/zh/design/compilation.mddocs/zh/design/template-resolver.mddocs/zh/dev/build.mddocs/zh/dev/contribution.mddocs/zh/dev/test-and-debug.mddocs/zh/guide/configuration.mddocs/zh/guide/quick-start.mddocs/zh/index.mdeditors/vscode/.eslintrc.jsoneditors/vscode/.vscode/extensions.jsoneditors/vscode/.vscode/launch.jsoneditors/vscode/.vscode/settings.jsoneditors/vscode/.vscode/tasks.jsoneditors/vscode/src/download.tseditors/vscode/src/extension.tseditors/vscode/src/feature/header.tseditors/vscode/src/feature/highlight.tseditors/vscode/src/setting.tseditors/vscode/src/test/extension.test.tseditors/vscode/tsconfig.jsoneditors/vscode/webpack.config.jseditors/zed/Cargo.tomlpixi.tomltests/data/clang_tidy/clice.tomltests/pyproject.tomltombi.tomlxmake.lua
💤 Files with no reviewable changes (5)
- docs/zh/guide/quick-start.md
- .clang-format
- docs/zh/design/template-resolver.md
- docs/zh/dev/build.md
- docs/zh/design/compilation.md
✅ Files skipped from review due to trivial changes (24)
- config/llvm-manifest.json
- editors/vscode/.vscode/extensions.json
- editors/vscode/src/feature/header.ts
- tests/pyproject.toml
- docs/zh/guide/configuration.md
- docs/en/dev/contribution.md
- .github/ISSUE_TEMPLATE/feature_request.md
- editors/vscode/.vscode/settings.json
- README.md
- docs/.vitepress/theme/sidebar.ts
- editors/vscode/src/test/extension.test.ts
- editors/vscode/src/download.ts
- editors/vscode/webpack.config.js
- docs/.vitepress/theme/index.ts
- .github/ISSUE_TEMPLATE/bug_report.md
- docs/.vitepress/theme/style.css
- tombi.toml
- editors/zed/Cargo.toml
- editors/vscode/.vscode/tasks.json
- editors/vscode/tsconfig.json
- docs/en/dev/test-and-debug.md
- docs/zh/index.md
- docs/en/index.md
- editors/vscode/src/feature/highlight.ts
🧰 Additional context used
🧬 Code graph analysis (4)
xmake.lua (1)
include/Test/Tester.h (1)
add_files(30-33)
editors/vscode/src/extension.ts (2)
editors/vscode/src/setting.ts (1)
getSetting(10-34)editors/vscode/src/download.ts (1)
ensureServerBinary(17-144)
docs/.vitepress/config.ts (1)
docs/.vitepress/theme/sidebar.ts (1)
genSidebar(5-45)
editors/vscode/src/setting.ts (1)
tests/conftest.py (1)
executable(40-53)
🪛 markdownlint-cli2 (0.18.1)
docs/zh/dev/test-and-debug.md
75-75: Dollar signs used before commands without showing output
(MD014, commands-show-output)
76-76: Dollar signs used before commands without showing output
(MD014, commands-show-output)
77-77: Dollar signs used before commands without showing output
(MD014, commands-show-output)
⏰ 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: build (macos-15, releasedbg)
- GitHub Check: build (windows-2025, releasedbg)
- GitHub Check: build (macos-15, debug)
- GitHub Check: build (ubuntu-24.04, releasedbg)
- GitHub Check: build (ubuntu-24.04, debug)
- GitHub Check: build (windows-2025, RelWithDebInfo)
- GitHub Check: build (ubuntu-24.04, Debug)
- GitHub Check: build (macos-15, Debug)
🔇 Additional comments (20)
docs/zh/dev/contribution.md (1)
9-12: LGTM!Minor formatting improvement adding a blank line for better readability in the Code Style section.
editors/vscode/.vscode/launch.json (1)
5-17: LGTM!Formatting-only changes to align with project-wide style consistency. The launch configuration remains functionally unchanged.
docs/en/guide/configuration.md (1)
38-38: LGTM!Minor formatting adjustment for the
<br>tag indentation.tests/data/clang_tidy/clice.toml (2)
1-1: Verify the intended header format.The header
## # clice configurationappears unusual for a TOML file. Typically, TOML comments use a single#prefix. Was this intended to be# clice configurationor is this a specific template format?
3-5: LGTM!Good addition of a descriptive comment explaining the
clang_tidysetting.docs/zh/dev/test-and-debug.md (2)
54-66: LGTM!The JSONC configuration block formatting is clean and readable with appropriate comments.
73-78: LGTM!Formatting improvements to the shell command block.
Note: The static analysis tool flags the
$prefix (MD014), but this is a common convention in documentation to clearly indicate shell commands. The current style is acceptable.docs/.vitepress/config.ts (1)
1-38: LGTM!The VitePress configuration is well-structured with proper nesting of
localesandthemeConfig. The switch to double-quoted strings aligns with the project-wide formatting standardization. Functional behavior remains unchanged.editors/vscode/src/setting.ts (1)
1-34: LGTM!Formatting-only changes (double quotes, semicolons, spacing) for consistency with project style. The function logic and validation behavior remain unchanged.
xmake.lua (2)
148-151: Source paths successfully migrated to new locations.The clice target correctly references
src/clice.ccand unit_tests correctly referencestests/unit/unit_tests.cc. Both new source files exist, and old paths have been removed.
173-177: Unit tests source path reorganization looks good.Moving
unit_tests.ccfrombin/totests/unit/provides better organization by keeping test files together. Thetests/unit/**.cppglob pattern correctly matches 45 test files across the organized subdirectories (AST, Async, Compiler, Feature, Index, Server, Support, Test).editors/vscode/src/extension.ts (2)
1-11: LGTM!The imports are well-organized with consistent double-quote formatting. The separation of named imports from
vscode(line 3) alongside the namespace import (line 2) is a reasonable pattern for accessing both common utilities and the full API.
15-21: LGTM!The activation flow, command registration, server option construction, and language client setup are well-structured. The conditional handling for pipe vs. socket modes with proper error messaging is clean, and the auto-download fallback via
ensureServerBinaryis a good UX improvement.Also applies to: 23-86
.github/workflows/check-format.yml (1)
10-12: Permissions are correctly scoped.The
pull-requests: writepermission is required for reviewdog to post inline suggestions, andcontents: readfollows the principle of least privilege.docs/clice.toml (1)
11-26: LGTM!The configuration is well-documented with clear comments explaining each option. The use of
${workspace}variable for paths and the catch-all**/*pattern for rules are appropriate defaults.Also applies to: 30-47
editors/vscode/.eslintrc.json (1)
1-24: LGTM!The ESLint configuration is well-structured. The pattern of disabling the base
semirule (line 21) while enabling@typescript-eslint/semi(line 17) is correct to avoid conflicts. Thenaming-conventionrule for imports aligns with the double-quote and consistent styling applied across the extension files.pixi.toml (3)
53-74: LGTM!The formatting tasks are well-organized with clear separation by file type. Using
fdfor file discovery with appropriate extensions and exclusions (e.g.,-E package-lock.json) is a good pattern. Theformatmeta-task withdepends-onprovides a convenient single command to run all formatters.
81-85: LGTM!The task definitions are well-structured with proper use of array-form commands for complex invocations. The CI tasks with proper dependency chains and the environment definitions are clean.
Also applies to: 87-99, 101-128, 130-154
45-51: No issue withclang-formatversion 21.1.7.Version
21.1.7is available in conda-forge and is a valid pin. No action required.Likely an incorrect or invalid review comment.
.prettierrc.yaml (1)
1-23: LGTM!The Prettier configuration is well-structured and aligns with the formatting changes across the PR. Key decisions are sound:
tabWidth: 4for code files,tabWidth: 2for config/data filessingleQuote: falsematches the double-quote standardization in TypeScript files- Including
.clang-formatand.clang-tidyin YAML overrides correctly handles these YAML-based config files
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/check-format.yml (1)
41-47: Previous feedback addressed! Consider adjusting strictness settings.The previous review concerns have been fully resolved—the action is now pinned to
@v1.24.0and usesfail_levelinstead of the deprecatedfail_on_error.However, you may want to consider whether
fail_level: anyandfilter_mode: nofilteralign with your team's workflow:
fail_level: anywill fail the workflow on any suggestion (info, warning, or error), which is quite strict.filter_mode: nofilterwill show suggestions for the entire repository, not just changed files.For a more lenient approach, consider
fail_level: errorandfilter_mode: diff_contextto fail only on errors and focus on changed files.🔎 Alternative configuration for less strict checks
- name: Suggest changes if: github.event_name == 'pull_request' uses: reviewdog/action-suggester@v1.24.0 with: tool_name: "fmt" - fail_level: any - filter_mode: nofilter + fail_level: error + filter_mode: diff_context
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.coderabbit.yaml.github/workflows/check-format.yml.pre-commit-config.yaml
💤 Files with no reviewable changes (1)
- .pre-commit-config.yaml
✅ Files skipped from review due to trivial changes (1)
- .coderabbit.yaml
⏰ 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: build (macos-15, debug)
- GitHub Check: build (macos-15, releasedbg)
- GitHub Check: build (ubuntu-24.04, debug)
- GitHub Check: build (windows-2025, releasedbg)
- GitHub Check: build (ubuntu-24.04, releasedbg)
- GitHub Check: build (macos-15, Debug)
- GitHub Check: build (windows-2025, RelWithDebInfo)
- GitHub Check: build (ubuntu-24.04, Debug)
🔇 Additional comments (2)
.github/workflows/check-format.yml (2)
9-14: LGTM! Permissions follow least-privilege principle.The permissions block correctly grants the minimum required rights for the reviewdog suggester and autocorrect actions to function properly.
31-47: Verify the interaction betweencontinue-on-errorandfail_level: any.The workflow allows both the formatter (line 33) and autocorrect (line 39) steps to continue on error, but then the suggester (line 46) will fail the workflow on any suggestion. This pattern collects all formatting issues before failing, which is reasonable, but please confirm this is the intended behavior.
If you want the workflow to fail immediately when formatting tools encounter errors, remove
continue-on-error: truefrom the earlier steps.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.