Skip to content

Conversation

thaJeztah
Copy link
Member

Besides the docker CLI, compose also uses this for the classic builder, so we need a place to put this; it's likely out of scope for the client itself, so we may as well put it here for now.

Removes direct imports of github.com/docker/docker/builder in the image package, to be moved later.

- Human readable description for the release notes

Go SDK: cli/command/image/build: add a `DetectContextType` utility to detect the type of build-context.

- A picture of a cute animal (not mandatory but encouraged)

@thaJeztah thaJeztah added status/2-code-review area/builder area/builder/classic-builder kind/refactor PR's that refactor, or clean-up code area/go-sdk Changes affecting the Go SDK labels Jul 16, 2025
@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 21.73913% with 36 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
cli/command/image/build.go 32.25% 18 Missing and 3 partials ⚠️
cli/command/image/build/context_detect.go 0.00% 15 Missing ⚠️

📢 Thoughts on this report? Let us know!

@thaJeztah thaJeztah force-pushed the cleanup_build_context branch 2 times, most recently from 818a642 to 246be82 Compare July 16, 2025 13:30
@thaJeztah thaJeztah marked this pull request as ready for review July 16, 2025 13:31
@thaJeztah thaJeztah added this to the 29.0.0 milestone Jul 16, 2025
Copy link
Collaborator

@vvoland vvoland left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

type ContextType string

const (
ContextTypeStdIn ContextType = "stdin" // ContextTypeStdIn indicates that the build-context is a TAR archive passed through STDIN.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: IMO StdIn looks weird and isn't consistent with the general naming (like os.Stdin).

Suggested change
ContextTypeStdIn ContextType = "stdin" // ContextTypeStdIn indicates that the build-context is a TAR archive passed through STDIN.
ContextTypeStdin ContextType = "stdin" // ContextTypeStdin indicates that the build-context is a TAR archive passed through STDIN.

(just an opinion though)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, yeah, can change; I always read its as standard-in / standard-out, standard-err, not as a single word, but Golang uses os.Stdin, so it makes sense to follow that.

Let me update.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done! Will merge when CI finishes.

Removes direct imports of github.com/docker/docker/builder in
the image package, to be moved later.

Signed-off-by: Sebastiaan van Stijn <[email protected]>
@thaJeztah thaJeztah force-pushed the cleanup_build_context branch from 246be82 to 260f1db Compare July 16, 2025 13:50
@thaJeztah thaJeztah merged commit b5a9392 into docker:master Jul 16, 2025
87 checks passed
@thaJeztah thaJeztah deleted the cleanup_build_context branch July 16, 2025 13:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants