-
Notifications
You must be signed in to change notification settings - Fork 2k
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
cli/command: move prompt utilities to separate package #5952
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #5952 +/- ##
==========================================
- Coverage 59.13% 59.12% -0.02%
==========================================
Files 355 356 +1
Lines 29744 29750 +6
==========================================
Hits 17590 17590
- Misses 11179 11186 +7
+ Partials 975 974 -1 🚀 New features to boost your workflow:
|
afe1ab9
to
12c00f8
Compare
12c00f8
to
d75384e
Compare
@@ -9,6 +9,7 @@ import ( | |||
"github.com/docker/cli/cli" | |||
"github.com/docker/cli/cli/command" | |||
"github.com/docker/cli/cli/command/completion" | |||
"github.com/docker/cli/internal/prompt" |
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 we have it in pkg
so we can use it in buildx repo (https://github.com/docker/buildx/blob/646df6d4a06848c675abf110dd51c2623b517590/commands/util.go#L15-L57)?
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.
Ah, right, but that would probably not work currently? i.e., that fork was because of docker/buildx#2359 (comment), or was that part of it reverted?
My thinking of keeping it "internal" for now at least, was because there may still be some work to do to get a stable "API" for these. There's some functions that currently depend on command.Stream
(or equivalent), and ideally they'd be more generic (I'm a bit on the fence on the Stream
wrappers; they're useful, but perhaps we can do without so that we can pass a regular os.StdIn
/ os.StdErr
etc).
66ff9be
to
d9b0d1b
Compare
d9b0d1b
to
4330eb1
Compare
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
4330eb1
to
aa74eb4
Compare
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
aa74eb4
to
b74b7b3
Compare
Rebase was for conflict in the imports; CI is happy again, so bringing this one in |
See individual commits for details; may be doing some follow-ups after this.
- Human readable description for the release notes
- A picture of a cute animal (not mandatory but encouraged)