Remove most ctr refs from CLI #874
Merged
Conversation
This file contains 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
sondavidb
force-pushed
the
remove-ctr-refs-from-cli
branch
from
3180631 to
58e8b1f
Compare
Kern--
reviewed
Oct 20, 2023
sondavidb
force-pushed
the
remove-ctr-refs-from-cli
branch
6 times, most recently
from
c775af5 to
f7ce392
Compare
|
Overall LGTM, besides the small nit. Can we update the commit message to mention which specific commands still reference the |
turan18
reviewed
Oct 27, 2023
sondavidb
force-pushed
the
remove-ctr-refs-from-cli
branch
from
f7ce392 to
87b9248
Compare
|
Removed all references except from "image rpull" and "run". "run" is entirely delegated to ctr and thus probably not worth porting. "image rpull" uses more than just flags and might be a tad more difficult to port over. Signed-off-by: David Son <[email protected]>
sondavidb
force-pushed
the
remove-ctr-refs-from-cli
branch
from
87b9248 to
b19950e
Compare
Kern--
approved these changes
Oct 30, 2023
turan18
approved these changes
Oct 31, 2023
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.
Issue #, if available:
#841 (partial fix)
Description of changes:
Removed many references to ctr. Only things not removed were the
runand theimage rpullcommands. Both were more challenging to port over —runis entirely delegated to ctr and may not even be worth the effort to port over, whileimage rpulluses more than just the containerd client, which might require larger scale changes should we focus on this later. All other references to ctr should be gone.Unsure if the place I put client.go adhered to proper structure. It made sense to me given that every other file in every other folder was relevant to a command, so the
internalfolder made the most sense to me for the code for creating a new containerd client.Testing performed:
Minimal testing on some basic operations (create, push, ztoc, ls) and ensured they remained functionally the same. I suppose the testing suite will perform some more robust tests as they use the CLI.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.