Skip to content

Move and refactor integration-cli/registry to internal/test#36839

Merged
thaJeztah merged 1 commit into
moby:masterfrom
vdemeester:integration-registry-refactoring
Apr 13, 2018
Merged

Move and refactor integration-cli/registry to internal/test#36839
thaJeztah merged 1 commit into
moby:masterfrom
vdemeester:integration-registry-refactoring

Conversation

@vdemeester
Copy link
Copy Markdown
Member

  • Move the code from integration-cli to internal/test.
  • Use testingT and assert when creating the registry.

🦁

Signed-off-by: Vincent Demeester vincent@sbr.pm

Copy link
Copy Markdown
Member

@cpuguy83 cpuguy83 left a comment

Choose a reason for hiding this comment

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

LGTM

Comment thread internal/test/registry/registry.go Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nit, WaitReady seems more appropriate here.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

😻

@cpuguy83
Copy link
Copy Markdown
Member

14:14:22 internal/test/registry/ops.go:22:6:warning: func name will be used as registry.RegistryURL by other packages, and that stutters; consider calling this URL (golint)

@vdemeester vdemeester force-pushed the integration-registry-refactoring branch from 5c5329d to 29a6192 Compare April 12, 2018 14:23
Copy link
Copy Markdown
Member

@yongtang yongtang left a comment

Choose a reason for hiding this comment

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

LGTM

- Move the code from `integration-cli` to `internal/test`.
- Use `testingT` and `assert` when creating the registry.

Signed-off-by: Vincent Demeester <vincent@sbr.pm>
@vdemeester vdemeester force-pushed the integration-registry-refactoring branch from 29a6192 to 66de2e6 Compare April 13, 2018 08:46
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 13, 2018

Codecov Report

❗ No coverage uploaded for pull request base (master@544ec09). Click here to learn what that means.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master   #36839   +/-   ##
=========================================
  Coverage          ?   34.98%           
=========================================
  Files             ?      614           
  Lines             ?    45673           
  Branches          ?        0           
=========================================
  Hits              ?    15978           
  Misses            ?    27600           
  Partials          ?     2095

Copy link
Copy Markdown
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@thaJeztah thaJeztah merged commit a9f502d into moby:master Apr 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants