re-introduced support for port numbers in docker registry URL

docker / cli

The Docker CLI

Apache License 2.0

4.74k stars 1.88k forks source link

re-introduced support for port numbers in docker registry URL #5195

Closed CarstonSchilds closed 1 week ago

CarstonSchilds commented 1 week ago

- What I did

this is a fix for a regression introduced by https://github.com/docker/cli/pull/3599
re-introduced support for port numbers in docker registry URL
fixes https://github.com/docker/cli/issues/5194

- How to verify it

added new test cases and ran docker buildx bake test

- Description for the changelog

Fix a regression that caused port numbers to be ignored when parsing a Docker registry URL.

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

codecov-commenter commented 1 week ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 61.43%. Comparing base (ffb8425) to head (2380481).

Additional details and impacted files

```diff @@ Coverage Diff @@ ## master #5195 +/- ## ========================================== + Coverage 60.96% 61.43% +0.46% ========================================== Files 295 298 +3 Lines 20789 20799 +10 ========================================== + Hits 12674 12777 +103 + Misses 7198 7109 -89 + Partials 917 913 -4 ```

thaJeztah commented 1 week ago

🤦 Thanks! I missed that https://github.com/docker/cli/pull/3599 was merged, and looking at my last push there, I suspect I found some local changes that may have been WIP 😞

thaJeztah commented 1 week ago

Actually, ignore my comments for now; I guess we didn't do this in the old code, so we can make those fixes in a follow-up; I opened a follow-up PR; https://github.com/docker/cli/pull/5196

thaJeztah commented 1 week ago

@vvoland @laurazard PTAL