refactor: move config module from meta_cli to meta_core - #3
Conversation
Config (parse JSON/YAML, normalize structs, walk meta trees) is pure library code that belongs in the shared foundation crate. This fixes the inverted dependency where meta_git_lib, meta_rust_cli, and meta_mcp depended on meta_cli just for config parsing. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
WalkthroughAdds dependency Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Cargo.toml (1)
9-15:⚠️ Potential issue | 🟠 MajorConsider replacing serde_yaml with an actively maintained alternative.
The crate is deprecated and no longer maintained as of March 2024. While version 0.9.x is not affected by known security advisories (the DoS issue from RUSTSEC-2018-0005 was patched in 0.8.4+), using an unmaintained dependency introduces long-term maintenance risk. Evaluate switching to
serde_ymlor another maintained YAML serialization library.
🤖 Fix all issues with AI agents
In `@src/config.rs`:
- Around line 361-384: In check_orphan_status, normalize the path produced from
meta_dir.strip_prefix(parent_meta_dir) (the relative_str used for comparison
against flat_paths) to use forward slashes so Windows backslashes won't mismatch
the '/'-separated entries produced by flatten_meta_tree; update the code that
computes relative_str (or derive a new normalized_relative string) to join the
Path components with "/" (or replace '\' with '/') before the
flat_paths.iter().any(...) comparison so tracked repos are detected correctly
and the suggested_key logic remains unchanged.
…paths Address CodeRabbit review feedback: - Replace deprecated serde_yaml (unmaintained since March 2024) with serde_yml, a maintained fork with compatible API - Normalize path separators in check_orphan_status to use forward slashes, preventing false orphan detection on Windows Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Fixed the CodeRabbit review feedback in 6b82965:
|
CI was failing because the repo uses workspace-inherited fields (version.workspace = true) but was checked out in isolation without a workspace root. Add proper workspace setup (checkout into subdirectory, create workspace Cargo.toml) and smart branch cloning that tries the PR branch name first before falling back to main. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@Cargo.toml`:
- Line 14: The Cargo.toml dependency serde_yml is unsafe/unmaintained; replace
it with a maintained YAML crate such as serde_yaml_ng or serde_norway by
updating the dependency entry (swap the serde_yml = "0.0.12" line to the chosen
crate and version), run cargo update/cargo build to fetch the new crate, and
adjust any serde YAML usage in the codebase to the new crate's API (most code
using serde_yml should work with serde_yaml_ng, but if you choose serde_norway,
adapt import paths and any minor API differences accordingly).
In `@src/config.rs`:
- Around line 11-105: The public types claim a repo is required but
ProjectEntry::Extended stores repo as Option<String>, ProjectInfo.repo is
Option<String>, and has_no_repo() implies no-repo entries are permitted; update
the API docs and inline comments to reflect that repo is optional and local-only
projects are supported (adjust comments on ProjectEntry, ProjectEntry::Extended,
ProjectInfo, and the doc string above MetaConfig) so downstream users aren’t
misled, or alternatively enforce a required repo by changing
ProjectEntry::Extended.repo and ProjectInfo.repo to String and
removing/adjusting has_no_repo(); pick one approach and make the comments and
types consistent (reference symbols: ProjectEntry::Extended, ProjectInfo,
has_no_repo()).
- Around line 167-222: The ProjectInfo.path values built in parse_meta_config
can contain backslashes from config files and should be normalized to forward
slashes to match filesystem-derived paths; update the mapping inside
parse_meta_config (the closure that builds ProjectInfo in the .map over
config.projects) to normalize resolved_path/name by replacing backslashes with
'/' (e.g., resolved_path.replace('\\','/')) before assigning to ProjectInfo.path
so both the Simple branch and the Extended { path, ... } branch produce
normalized paths.
- Replace serde_yml (RUSTSEC-2025-0068: unsound, unmaintained) with serde_yaml_ng (maintained drop-in replacement) - Fix doc comments: clarify repo field is optional for local-only projects - Normalize project paths (backslash to forward slash) for Windows compat - Fix CI script injection: move github.head_ref to env: instead of inline shell interpolation - Add clone failure handling and branch logging in CI Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary
configmodule (JSON/YAML parsing, project structs, meta tree walking) frommeta_clitometa_coremeta_git_lib,meta_rust_cli, andmeta_mcpdepended onmeta_clijust for config parsingserde_yamlwith maintained forkserde_ymlcheck_orphan_statusfor Windows compatibilityMerge order
Merge this PR first — all other repos in this coordinated refactor depend on it:
refactor/move-config-to-meta-corerefactor/move-config-to-meta-corerefactor/move-config-to-meta-corerefactor/move-config-to-meta-corerefactor/move-config-to-meta-coreTest plan
cargo build --workspacepassescargo test --workspacepassesbats tests/git.bats— 51/51 passbats tests/plugin_install.bats— 15/15 passgrep -r "meta_cli::config" --include="*.rs"returns no matches🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Chores