Skip to content

Conversation

VihasMakwana
Copy link
Contributor

@VihasMakwana VihasMakwana commented Aug 1, 2025

Description

This POC PR implements the merging of lists by utilizing the yaml tags. This method makes very little changes to public API and completely relies on the information provided from the confi files.

We following these steps:

  1. We go through the yaml tree and search for tags.
  2. We keep track of current path and record the path to a "map", if user has specified merging tag.
  3. Once the yaml tree is traversed, we go through recorded paths and merge the lists for those paths.

Relates #13256

Copy link

codecov bot commented Aug 1, 2025

Codecov Report

❌ Patch coverage is 93.10345% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.88%. Comparing base (74cb2f3) to head (46b4be3).
⚠️ Report is 137 commits behind head on main.

Files with missing lines Patch % Lines
confmap/merge.go 91.78% 3 Missing and 3 partials ⚠️

❌ Your patch check has failed because the patch coverage (93.10%) is below the target coverage (95.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #13551      +/-   ##
==========================================
- Coverage   91.50%   90.88%   -0.63%     
==========================================
  Files         526      599      +73     
  Lines       29436    31460    +2024     
==========================================
+ Hits        26936    28592    +1656     
- Misses       1971     2333     +362     
- Partials      529      535       +6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

- name: merge-mode-default
configs:
-
- |
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume this is only needed for the test cases and not in an actual config file?

Copy link
Contributor Author

@VihasMakwana VihasMakwana Aug 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, only for test file. I changed this because I wanted the config to be a "string" and not a map.

Copy link
Contributor Author

@VihasMakwana VihasMakwana Aug 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason for doing this:

If we don't specify this as a "string", the yaml tags would get lost in json translation.
In real world configuration, this would not happen because of obvious reasons (we load config from yaml file and tags are preserved).

@evan-bradley evan-bradley changed the title [confma][PoC] - Showcase the use of yaml tags for merge append mode [confmap][PoC] - Showcase the use of yaml tags for merge append mode Aug 8, 2025
Copy link
Contributor

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions bot added the Stale label Aug 23, 2025
Copy link
Contributor

github-actions bot commented Sep 7, 2025

Closed as inactive. Feel free to reopen if this PR is still being worked on.

@github-actions github-actions bot closed this Sep 7, 2025
@mx-psi mx-psi reopened this Sep 8, 2025
@github-actions github-actions bot removed the Stale label Sep 9, 2025
Copy link
Contributor

This PR was marked stale due to lack of activity. It will be closed in 14 days.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants