Repository navigation
Deterministic sort - #5
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughThe import scanner records declaration metadata, classifies member imports, resolves aliases, and sorts imports deterministically. The sorter groups import bands, preserves comment lines, updates the version, and adds tests and guidance. ChangesImport ordering
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Source
participant ast_scan
participant zsort
participant Output
Source->>ast_scan: Analyze import declarations
ast_scan->>ast_scan: Resolve aliases and classify import bands
ast_scan-->>zsort: Return sorted import records
zsort->>Output: Emit imports, separators, and comment lines
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
AGENTS.md (1)
3-9: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMake the dual-version validation procedure explicit.
Each listed
zigcommand runs only the executable onPATH. After Line 1 maps binary names to versions, invoke every gate and bothtest/consumergates with each resolved Zig 0.15.2 and Zig 0.16 binary. Otherwise the procedure can validate only one version.Proposed guidance update
-Lint gates: for both 0.15.2 and 0.16 -- zig build test -- zig build check-imports -- zig fmt --check src -- zlint --deny-warnings -- zwanzig src build.zig -- also verify build and check-imports still work for the consumer in test/consumer +For each binary mapped above to Zig 0.15.2 and Zig 0.16, run: +- `$zig_bin build test` +- `$zig_bin build check-imports` +- `$zig_bin fmt --check src` +- `zlint --deny-warnings` +- `zwanzig src build.zig` +- `(cd test/consumer && $zig_bin build && $zig_bin build check-imports)`As per coding guidelines, “For both Zig 0.15.2 and Zig 0.16, all lint and validation gates must pass.” Based on learnings, verify which available local Zig binaries exist and determine the version of each.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@AGENTS.md` around lines 3 - 9, Update the validation procedure in AGENTS.md to explicitly resolve and identify the available Zig 0.15.2 and Zig 0.16 binaries, then run every listed gate—including all test/consumer build and check-imports checks—once with each version-specific binary rather than relying on the PATH default.Sources: Coding guidelines, Learnings
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@README.md`:
- Around line 121-127: Update the import-group description near the ordering
list to document that blank lines separate every change in import class or
import kind: plain and member groups are emitted separately for std/builtin,
third-party, and local imports, followed by the alias group ordered std/builtin
→ third-party → local without blank lines. Replace the “four bands” framing with
wording that accurately reflects these emitted groups.
In `@src/ast_scan.zig`:
- Around line 339-343: Update the alias classification around isAliasChain to
accept only identifier-only field chains, rejecting postfix expressions such as
calls and index access before they reach lineBlockEnd and import rebuilding. Add
regression coverage for both factory().Config and items[0].Config to ensure
neither is treated as an alias.
- Around line 284-296: Replace the source-text parsing in declName with
AST-based name extraction: use tree.tokenSlice(vd.ast.mut_token + 1) so modifier
whitespace, including tabs, is handled correctly. Update the function or its
callers to receive the relevant tree and variable declaration context, and
remove the manual modifier and identifier scanning.
---
Nitpick comments:
In `@AGENTS.md`:
- Around line 3-9: Update the validation procedure in AGENTS.md to explicitly
resolve and identify the available Zig 0.15.2 and Zig 0.16 binaries, then run
every listed gate—including all test/consumer build and check-imports
checks—once with each version-specific binary rather than relying on the PATH
default.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7017e12b-5478-4968-99a6-27a038813e8b
📒 Files selected for processing (5)
AGENTS.mdREADME.mdsrc/ast_scan.zigsrc/zsort.zigsrc/zsort_test.zig
Summary by CodeRabbit
New Features
Bug Fixes
Documentation