fix: clarify coalesce warning messages#32226
Open
devShaik010 wants to merge 1 commit into
Open
Conversation
Signed-off-by: devShaik010 <20dpcs044hy@manuu.edu.in>
pull-request-size
Bot
added
the
size/S
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Updates coalescing warning messages to use “map/non-map” terminology (and include offending values) and aligns tests with the new warning text.
Changes:
- Updated
coalesceValueswarning when encountering a non-table value to include the value and use “map/non-map” wording. - Updated
coalesceTablesFullKeywarnings to use “map/non-map” wording. - Adjusted unit tests to assert against the updated warning strings.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| pkg/chart/common/util/coalesce.go | Changes user-facing warning strings emitted during values coalescing. |
| pkg/chart/common/util/coalesce_test.go | Updates assertions to match the new warning message text. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // the warning | ||
| if val != nil { | ||
| printf("warning: skipped value for %s.%s: Not a table.", subPrefix, key) | ||
| printf("warning: skipped value for %s.%s: expected a map, got non-map value (%v)", subPrefix, key, val) |
Comment on lines
+350
to
+353
| printf("warning: cannot overwrite map with non-map for %s (%v)", fullkey, val) | ||
| } | ||
| } else if istable(dv) && val != nil { | ||
| printf("warning: destination for %s is a table. Ignoring non-table value (%v)", fullkey, val) | ||
| printf("warning: destination for %s is a map. Ignoring non-map value (%v)", fullkey, val) |
Author
|
Hi @TerryHowe, could you take a review when you get a chance? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR updates Helm's coalesce warnings to use clearer wording for map value conflicts.
Why
This makes the warning output easier for Helm users to understand and addresses the confusion described in the linked issue.
Testing
Could not run the package test in this environment. The local machine does not have Go installed, and the Docker daemon is not running.
Closes #11118