Skip to content

Implement WithDefaultContextStoreConfig() DockerCliOption#2992

Merged
thaJeztah merged 2 commits intodocker:masterfrom
thaJeztah:refactor_default_context_store_config
Mar 3, 2022
Merged

Implement WithDefaultContextStoreConfig() DockerCliOption#2992
thaJeztah merged 2 commits intodocker:masterfrom
thaJeztah:refactor_default_context_store_config

Conversation

@thaJeztah
Copy link
Member

@thaJeztah thaJeztah commented Mar 3, 2021

Just a minor refactor to make this slightly cleaner

@thaJeztah
Copy link
Member Author

thaJeztah commented Oct 7, 2021

@silvin-lubecki ptal

thaJeztah added 2 commits March 3, 2022 14:47
@thaJeztah
NewAPIClientFromFlags: rename variable to not collide with import
d35b50c 
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah
Implement WithDefaultContextStoreConfig() DockerCliOption
cc08fc1 
Just a minor refactor to make this slightly cleaner

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah thaJeztah force-pushed the refactor_default_context_store_config branch from fe7563f to cc08fc1 Compare March 3, 2022 13:48
rumpl
rumpl approved these changes Mar 3, 2022
View reviewed changes
Copy link
Member

@rumpl rumpl left a comment

Choose a reason for hiding this comment

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

👍

@thaJeztah thaJeztah added this to the 21.xx milestone Mar 3, 2022
@thaJeztah thaJeztah merged commit c61452c into docker:master Mar 3, 2022
@thaJeztah thaJeztah deleted the refactor_default_context_store_config branch March 3, 2022 15:25
@thaJeztah
Copy link
Member Author

thaJeztah commented Mar 3, 2022

FWIW, while rebasing this branch, I saw that RegisterDefaultStoreEndpoints() is now unused in our code, and DefaultContextStoreConfig() could potentially be un-exported. However, both are used by Buildx, so we need to verify if that's using it for the correct reasons (or testing the now deprecated Kubernetes contexts?)

https://github.com/docker/buildx/blob/6efcee28d59e1aa4d935e8d4055ca5c0edff0a4b/driver/kubernetes/context/load_test.go#L19

https://github.com/docker/buildx/blob/6efcee28d59e1aa4d935e8d4055ca5c0edff0a4b/driver/kubernetes/context/load.go#L41

/cc @crazy-max

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rumpl rumpl rumpl approved these changes

@silvin-lubecki silvin-lubecki Awaiting requested review from silvin-lubecki

+1 more reviewer

@ndeloof ndeloof ndeloof approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

area/context status/2-code-review

Projects

None yet

Milestone

23.0.0

Development

Successfully merging this pull request may close these issues.

3 participants

@thaJeztah @rumpl @ndeloof