Skip to content
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: remove unused args from ResolveDefaultContext() (step 2) #3510

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

thaJeztah
Copy link
Member

- Description for the changelog

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

@thaJeztah thaJeztah added status/2-code-review area/context kind/refactor PR's that refactor, or clean-up code labels Mar 30, 2022
@thaJeztah thaJeztah added this to the 21.xx milestone Mar 30, 2022
@thaJeztah thaJeztah force-pushed the context_cleanup_part1 branch from 298b953 to 7770910 Compare March 30, 2022 16:28
@codecov-commenter
Copy link

codecov-commenter commented Mar 30, 2022

Codecov Report

Merging #3510 (13bccd3) into master (45f62db) will increase coverage by 0.05%.
The diff coverage is 94.73%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3510      +/-   ##
==========================================
+ Coverage   59.72%   59.77%   +0.05%     
==========================================
  Files         287      287              
  Lines       24832    24806      -26     
==========================================
- Hits        14831    14828       -3     
+ Misses       9115     9092      -23     
  Partials      886      886              

@thaJeztah
Copy link
Member Author

@silvin-lubecki @rumpl ptal

@thaJeztah
Copy link
Member Author

@ndeloof @rumpl ptal

thaJeztah added a commit to thaJeztah/buildx that referenced this pull request Apr 8, 2022
just double-checking if docker/cli#3510 doesn't
break anything.

Signed-off-by: Sebastiaan van Stijn <[email protected]>
@@ -31,66 +29,32 @@ type ContextStoreWithDefault struct {
Resolver DefaultContextResolver
}

// EndpointDefaultResolver is implemented by any EndpointMeta object
// which wants to be able to populate the store with whatever their default is.
type EndpointDefaultResolver interface {
Copy link
Member Author

@thaJeztah thaJeztah Apr 8, 2022

Choose a reason for hiding this comment

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

Looks like this is used in buildx (see docker/buildx#1054); let me check how/why and if it's strictly needed

+ xx-go build -ldflags '-X github.com/docker/buildx/version.Version=9e61f69 -X github.com/docker/buildx/version.Revision=9e61f69a5b83c3d6ee5545e488d5ad430c894276 -X github.com/docker/buildx/version.Package=github.com/docker/buildx -w -s' -o /usr/bin/buildx ./cmd/buildx
# github.com/docker/buildx/driver/kubernetes/context
driver/kubernetes/context/load.go:31:7: undefined: command.EndpointDefaultResolver

@thaJeztah
Copy link
Member Author

let me move this temporarily back to draft, while we can discuss what to do with these options.

@thaJeztah thaJeztah marked this pull request as draft April 8, 2022 14:52
@thaJeztah thaJeztah force-pushed the context_cleanup_part1 branch from c735e08 to 13edf41 Compare July 21, 2022 16:12
@thaJeztah thaJeztah changed the title cli/command: remove unused args from ResolveDefaultContext() cli/command: remove unused args from ResolveDefaultContext() (step 2) Jul 21, 2022
@thaJeztah thaJeztah force-pushed the context_cleanup_part1 branch from 13edf41 to 624d62b Compare July 29, 2022 07:49
@thaJeztah thaJeztah modified the milestones: 23.0.0, 23.0.1 Feb 2, 2023
@thaJeztah thaJeztah modified the milestones: 23.0.1, v-next Feb 9, 2023
@thaJeztah thaJeztah force-pushed the context_cleanup_part1 branch from 624d62b to 6ea9cba Compare April 27, 2023 00:38
@thaJeztah thaJeztah modified the milestones: 24.0.0, 25.0.0 May 5, 2023
@thaJeztah thaJeztah force-pushed the context_cleanup_part1 branch from 6ea9cba to d358867 Compare May 5, 2023 23:34
this was added as part of 520be05, but
is not used anywhere, so we may as well simplify things a bit.

Signed-off-by: Sebastiaan van Stijn <[email protected]>
This was added in 4f14c49, in preparation
of 1433e27, but no longer used, so we
can remove it.

Signed-off-by: Sebastiaan van Stijn <[email protected]>
@thaJeztah thaJeztah force-pushed the context_cleanup_part1 branch from d358867 to 13bccd3 Compare November 20, 2023 11:08
@thaJeztah thaJeztah removed this from the 25.0.0 milestone Jan 19, 2024
@thaJeztah thaJeztah added this to the 26.0.0 milestone Jan 19, 2024
@vvoland vvoland modified the milestones: 26.0.0, 27.0.0 Mar 14, 2024
@vvoland vvoland modified the milestones: 27.0.0, v-future Jun 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/context kind/refactor PR's that refactor, or clean-up code status/2-code-review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants