Skip to content

use local ConvertToHostname() implementation - #3599

Merged
vvoland merged 1 commit into
docker:masterfrom
thaJeztah:use_local_ConvertToHostname
Jun 20, 2024
Merged

use local ConvertToHostname() implementation#3599
vvoland merged 1 commit into
docker:masterfrom
thaJeztah:use_local_ConvertToHostname

Conversation

@thaJeztah

Copy link
Copy Markdown
Member

Commit 27b2797 (#1642) added a local implementation of this function, so let's use the local variant to (slightly) reduce the dependency on moby's registry package.

Also made some minor cleanups.

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

@thaJeztahthaJeztah added status/2-code-review kind/refactor PR's that refactor, or clean-up code labels May 10, 2022
@codecov-commenter

codecov-commenter commented May 10, 2022

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 87.50000% with 1 line in your changes missing coverage. Please review.

Project coverage is 61.78%. Comparing base (925e7d6) to head (8376b3e).

Additional details and impacted files
@@ Coverage Diff @@## master #3599 +/- ##
==========================================
+ Coverage 61.76% 61.78% +0.01% 
==========================================
Files 297 297 Lines 20768 20768 ==========================================
+ Hits 12828 12831 +3 + Misses 7024 7023 -1 + Partials 916 914 -2 

@thaJeztah

Copy link
Copy Markdown
MemberAuthor

@rumpl@ndeloof ptal 🤗

@ndeloof

Copy link
Copy Markdown
Contributor

I don't think "Copied from github.com/docker/docker/registry.ConvertToHostname to reduce dependencies" makes our codebase better.

  1. we still (obviously) depend on docker/docker as the API is not an isolated project vs engine backend
  2. we now have two implementations of the same ConvertToHostname function to maintain (for sure, low maintenance expected here)

imho it would make more sense for such utility func to be shared between cli and engine within one of the api package. I'm not sure about the initial intent in #1642

@thaJeztah
thaJeztahforce-pushed the use_local_ConvertToHostname branch from ab2bdbc to 713297dCompareJune 13, 2024 22:05
Commit 27b2797 added a local implementation
of this function, so let's use the local variant to (slightly) reduce the
dependency on moby's registry package.
Also made some minor cleanups.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah
thaJeztahforce-pushed the use_local_ConvertToHostname branch from 713297d to 8376b3eCompareJune 13, 2024 22:25
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/refactorPR's that refactor, or clean-up codestatus/2-code-review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@thaJeztah@codecov-commenter@ndeloof@vvoland