-
Notifications
You must be signed in to change notification settings - Fork 129
Config remote sync command #4289
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
| } | ||
|
|
||
| // SaveFiles writes all file changes to disk. | ||
| func SaveFiles(ctx context.Context, b *bundle.Bundle, files []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.
Q: Does it make sense to use yamlsaver here? Not sure what the benefits would be
| } | ||
|
|
||
| // Check for key-value selector: [key='value'] | ||
| if key, value, ok := n.KeyValue(); ok { |
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 part is probably not efficient and should be part of the mapping, not a string converter. Keeping it as is for now, as this makes it more modular. I'll see if it needs to be updated in next PRs
|
Commit: ee2564d
19 interesting tests: 10 KNOWN, 7 RECOVERED, 1 SKIP, 1 flaky
Top 50 slowest tests (at least 2 minutes):
|
| // Example: "resources.jobs.my_job" -> type="jobs", name="my_job" | ||
| func parseResourceKey(resourceKey string) (resourceType, resourceName string, err error) { | ||
| parts := strings.Split(resourceKey, ".") | ||
| if len(parts) < 3 || parts[0] != "resources" { |
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.
Hardcoded length for now, I'll check in next PRs if it makes sense to make this more generic
| return fmt.Errorf("failed to detect changes: %w", err) | ||
| } | ||
|
|
||
| files, err := configsync.GenerateYAMLFiles(ctx, b, changes) |
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 could potentially be split into 2 separate phases: applyChanges and generateYaml. I will check what suits the best when I am working on preserving original YAML formatting
Changes
Why
Tests