cli icon indicating copy to clipboard operation
cli copied to clipboard

cli/command: use client.CurrentVersion()

Open thaJeztah opened this issue 3 years ago • 2 comments

Make sure the new CurrentVersion() function, which was added in a7e2c3ea1edd195df98cb62bc353fcbf034764cd (https://github.com/docker/cli/pull/3885), but it looks like I overlooked these uses of what it was replacing 😅

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

thaJeztah avatar Dec 05 '22 21:12 thaJeztah

Thanks! I want to have another look at this; there's many places left where we call this (through Client().ClientVersion()), but in most places the Client() is also needed for other purposes (so it wouldn't make much sense to replace it).

That said, I want to look what makes most sense for various locations;

  • Pass the whole DockerCLI around (which includes the Client() API Client
  • Or if we should exactly do the reverse; only pass the APIClient

The reason the whole DockerCLI is passed around (in MOST cases), is only to get the CLI's .Out() or .Err() for printing messages. Perhaps we should pass something through the context or use another approach.

thaJeztah avatar Dec 06 '22 09:12 thaJeztah

Codecov Report

Merging #3902 (c0b37c0) into master (45f62db) will not change coverage. The diff coverage is 0.00%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #3902   +/-   ##
=======================================
  Coverage   59.72%   59.72%           
=======================================
  Files         287      287           
  Lines       24832    24832           
=======================================
  Hits        14831    14831           
  Misses       9115     9115           
  Partials      886      886           

codecov-commenter avatar Nov 20 '23 11:11 codecov-commenter