make registry apply accept a list of files #1101
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1101 +/- ##
==========================================
- Coverage 68.26% 68.24% -0.02%
==========================================
Files 145 145
Lines 11984 11993 +9
==========================================
+ Hits 8181 8185 +4
- Misses 3096 3100 +4
- Partials 707 708 +1
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
cmd/registry/cmd/apply/apply.go
Outdated
| Args: cobra.NoArgs, | ||
| Long: "Apply YAML to the API Registry by files / folder names or stdin. " + | ||
| "Resources will be created if they don't exist yet. " + | ||
| "Multiple files may be specified in -f separated by commas or by repeating the -f flag." + |
There was a problem hiding this comment.
Does kubectl apply -f split on commas? I didn't know about that.
There was a problem hiding this comment.
No idea if kubectl does, but this does.
There was a problem hiding this comment.
If kubectl doesn't do it, would you mind removing that? It's duplicative of multiple -f options, so it's only a small usability improvement.
There was a problem hiding this comment.
I'll remove the comment, but that's just the way that cobra works.
| }) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| } | ||
| return patches.run(ctx, jobs) |
There was a problem hiding this comment.
Just double-checking: because we collect all the inputs before we call this, there's no sensitivity to the ordering of file names, right?
There was a problem hiding this comment.
What is your specific concern?
There was a problem hiding this comment.
I wouldn't want a child resource to fail to apply because its parent hasn't been applied yet. That's already addressed in patches.run() so since we're only callling patches.run() once for all the inputs, we should be good.
There was a problem hiding this comment.
Right. Nothing has changed wrt. how a file is added to patches.run() other than now multiples paths (and their subpaths) may be added before the run. That said, and as a separate issue, I don't see the dependency case checked in patch_test.go - or am I missing it?
There was a problem hiding this comment.
OK, sg. We might not be checking the dependency in the tests.
Fixes #1097