-
Notifications
You must be signed in to change notification settings - Fork 137
Make config-remote-sync patching logic more generic #4400
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
Commit: b694b69
18 interesting tests: 9 KNOWN, 5 SKIP, 4 RECOVERED
Top 50 slowest tests (at least 2 minutes):
|
|
|
||
| // normalizeValue converts values to plain Go types suitable for YAML patching | ||
| // by using SDK marshaling which properly handles ForceSendFields and other annotations. | ||
| func normalizeValue(v any) (any, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is moved as is from patch.go
Will check if it's needed and can it be replaced with dyn in following PRs
|
|
||
| type resolvedChanges map[string]*deployplan.ChangeDesc | ||
| // ApplyChangesToYAML generates YAML files for the given field changes. | ||
| func ApplyChangesToYAML(ctx context.Context, b *bundle.Bundle, fieldChanges []FieldChange) ([]FileChange, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was simplified to reduce nesting level, also bundle-specific logic was moved to diff.go and resolve.go
| } | ||
|
|
||
| // ResolveChanges resolves selectors and computes field path candidates for each change. | ||
| func ResolveChanges(ctx context.Context, b *bundle.Bundle, configChanges Changes) ([]FieldChange, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All path resolution is handled here now, I will update this to traverse path nodes in following PRs
denik
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you make PR description more concrete? Which functions/types were moved, which were changed, which replaced by something newer.
Changes
Small refactoring to simplify further changes. Mostly cosmetic - simplification, reduce nesting, moving DAB-specific logic from patcher. Also improved command output for easier debugging
Why
Preparation before the next PR, where I plan to properly handle CLI defaults
Tests