Conversation
michaelfig
requested review from
balopat,
dgageot,
nkubala,
priyawadhwa and
r2d4
as code owners
|
Notably, the touch-after-sync is avoided by using the Maybe controversially, I added the |
Codecov Report
@@ Coverage Diff @@
## master #1641 +/- ##
=========================================
- Coverage 46.95% 46.9% -0.06%
=========================================
Files 119 119
Lines 5143 5151 +8
=========================================
+ Hits 2415 2416 +1
- Misses 2480 2487 +7
Partials 248 248
Continue to review full report at Codecov.
|
r2d4
left a comment
r2d4
left a comment
There was a problem hiding this comment.
Looks pretty great - haven't tried it out yet but just a few comments.
| args = append(args, dst) | ||
| } | ||
| delete := exec.CommandContext(ctx, "kubectl", args...) | ||
| return []*exec.Cmd{delete} |
There was a problem hiding this comment.
Both copyFn and deleteFn can just return a single exec.Cmd? WDYT?
There was a problem hiding this comment.
No harm in leaving them as slices, but I'm indifferent.
| if len(synced) != len(files) { | ||
| return errors.New("couldn't sync all the files in " + ns) | ||
| } | ||
| if numSynced == 0 { |
There was a problem hiding this comment.
Not sure if this check makes sense anymore?
There was a problem hiding this comment.
I put this in to pass the "no copy" test at sync_test.go:410. If you want, both this and the test can be removed.
6 participants
This PR implements steps 1 and 2 of #1639.