Reapply YAML deep-merge fix on v5#2527
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
…YAML Previously, when merging multiple YAML documents, entries under the same top-level section (e.g., two different deployers under `deployers:`) would be lost because the shallow merge simply replaced the entire hash. This implements deep recursive merging so that different entries under the same section are properly combined.
9252f04 to
0d5f85e
Compare
|
@coderabbitai assess this PR size classification for the totality of the PR with the following criterias and report it in your comment: S/M/L PR Classification Guidelines:This guide helps classify merged pull requests by effort and complexity rather than just line count. The goal is to assess the difficulty and scope of changes after they have been completed. Small (S)Characteristics:
Review Effort: Would have taken 5-10 minutes Examples:
Medium (M)Characteristics:
Review Effort: Would have taken 15-30 minutes Examples:
Large (L)Characteristics:
Review Effort: Would have taken 45+ minutes Examples:
Additional Factors to ConsiderWhen deciding between sizes, also consider:
Notes:
|
Summary
deployers:— second one would silently clobber the first)crates/settings/src/yaml/emitter.rsso entries under the same top-level section are properly combinedChanges
crates/settings/src/yaml/emitter.rs: addsdeep_merge_hashhelper and updatesemit_documentsto use it; adds regression testTest plan
test_emit_deep_merges_hash_sectionstest passes🤖 Generated with Claude Code