Import DisplayablePorts from moby/moby source by vdemeester · Pull Request #236 · docker/cli

vdemeester · 2017-06-23T14:27:59Z

This has nothing to do in the api package for moby as it's just a
cli display function.

Related PR in moby/moby: moby/moby#33798

Signed-off-by: Vincent Demeester [email protected]

thaJeztah · 2017-06-23T14:43:27Z

Looks like CI (lint) isn't happy;

cli/command/formatter/container_test.go:649::warning: line is 216 characters (lll)
cli/command/formatter/container_test.go:649::warning: line is 216 characters (lll)
cli/command/formatter/container_test.go:649::warning: line is 216 characters (lll)
Exited with code 1

thaJeztah

one nit, but LGTM otherwise

thaJeztah · 2017-06-23T14:34:33Z

 	"strings"
 	"time"

+	"sort"


this should move up?

dnephin · 2017-06-23T16:01:48Z

+
+	for _, port := range cases {
+		actual := DisplayablePorts(port.ports)
+		if port.expected != actual {


assert.Equal()

dnephin · 2017-06-23T16:02:16Z

+	expected string
+}
+
+// DisplayablePorts


Comment can probably be removed 😄

oh lol, I just copied it, yeah it can :D

vdemeester · 2017-06-23T17:38:12Z

Updated, should be fixed 👼

codecov-io · 2017-06-23T17:38:14Z

Codecov Report

Merging #236 into master will increase coverage by 0.25%.
The diff coverage is 96.42%.

@@            Coverage Diff             @@
##           master     #236      +/-   ##
==========================================
+ Coverage   46.96%   47.21%   +0.25%     
==========================================
  Files         171      171              
  Lines       11501    11552      +51     
==========================================
+ Hits         5401     5454      +53     
+ Misses       5790     5787       -3     
- Partials      310      311       +1

dnephin · 2017-06-23T17:40:34Z

 	"github.com/stretchr/testify/require"
 )

+// nolint: lllf


lllf?

Is this on the wrong function? It used to pass.

ah dang it.. it's a typo on my part after running the lint 😱

oh right 😛 Updated :)

thaJeztah · 2017-06-23T18:09:22Z

You forgot to update https://github.com/docker/cli/pull/236/files#r123762476 😇

This has nothing to do in the `api` package for moby as it's just a cli display function. Signed-off-by: Vincent Demeester <[email protected]>

vdemeester · 2017-06-23T18:14:02Z

@thaJeztah oups, done 🎐

thaJeztah

LGTM, thanks!

dnephin

LGTM

Includes changes from; - Fix handling of remote "git@" notation (moby/moby#33696) - Move some `api` package functions away moby/moby#33798 (related to docker#236) - Update go-connections dependency moby/moby#33814 (already vendored in docker#238) - Set ping version even on error (moby/moby#33827) - Do not add duplicate platform information to service spec (moby/moby#33867) - Refactor MountPoint Setup function in volume.go (moby/moby#33890) Signed-off-by: Sebastiaan van Stijn <[email protected]>

Includes changes from; - Add a LastTagTime for images (moby/moby#31497) - Fix handling of remote "git@" notation (moby/moby#33696) - Move some `api` package functions away (moby/moby#33798) (related to docker#236) - Set ping version even on error (moby/moby#33827) - Do not add duplicate platform information to service spec (moby/moby#33867) - Refactor MountPoint Setup function in volume.go (moby/moby#33890) Signed-off-by: Sebastiaan van Stijn <[email protected]>

Includes changes from; - Add a LastTagTime for images (moby/moby#31497) - Fix handling of remote "git@" notation (moby/moby#33696) - Move some `api` package functions away (moby/moby#33798) (related to docker/cli#236) - Set ping version even on error (moby/moby#33827) - Do not add duplicate platform information to service spec (moby/moby#33867) - Refactor MountPoint Setup function in volume.go (moby/moby#33890) Signed-off-by: Sebastiaan van Stijn <[email protected]> Upstream-commit: 366d3ec971d8007c667e8d7dc8e35a346fb19539 Component: cli

Includes changes from; - Add a LastTagTime for images (moby/moby#31497) - Fix handling of remote "git@" notation (moby/moby#33696) - Move some `api` package functions away (moby/moby#33798) (related to docker#236) - Set ping version even on error (moby/moby#33827) - Do not add duplicate platform information to service spec (moby/moby#33867) - Refactor MountPoint Setup function in volume.go (moby/moby#33890) Signed-off-by: Sebastiaan van Stijn <[email protected]>

…nfigs-idempotence [17.09] sort secrets and configs to ensure idempotence

vdemeester requested review from cpuguy83, dnephin, mlaventure and thaJeztah June 23, 2017 14:27

GordonTheTurtle added the status/0-triage label Jun 23, 2017

vdemeester mentioned this pull request Jun 23, 2017

Move some api package functions away moby/moby#33798

Merged

vdemeester added status/2-code-review and removed status/0-triage labels Jun 23, 2017

thaJeztah requested changes Jun 23, 2017

View reviewed changes

dnephin reviewed Jun 23, 2017

View reviewed changes

vdemeester force-pushed the import-displayable-ports-from-moby branch from 0739f38 to f1c953c Compare June 23, 2017 17:38

vdemeester force-pushed the import-displayable-ports-from-moby branch from f1c953c to 5dc31fb Compare June 23, 2017 17:39

dnephin reviewed Jun 23, 2017

View reviewed changes

vdemeester force-pushed the import-displayable-ports-from-moby branch from 5dc31fb to eabff39 Compare June 23, 2017 18:05

Import DisplayablePorts from moby/moby source

9e142ca

This has nothing to do in the `api` package for moby as it's just a cli display function. Signed-off-by: Vincent Demeester <[email protected]>

vdemeester force-pushed the import-displayable-ports-from-moby branch from eabff39 to 9e142ca Compare June 23, 2017 18:12

thaJeztah approved these changes Jun 23, 2017

View reviewed changes

dnephin approved these changes Jun 23, 2017

View reviewed changes

dnephin merged commit 8b6196d into docker:master Jun 23, 2017

GordonTheTurtle added this to the 17.07.0 milestone Jun 23, 2017

vdemeester deleted the import-displayable-ports-from-moby branch June 23, 2017 19:02

vdemeester mentioned this pull request Jun 24, 2017

sirupsen/logrus is used in client code and its dependencies moby/moby#33813

Open

thaJeztah mentioned this pull request Jul 3, 2017

Bump moby to 87df0e533b619c088091fd1e2310e92bb9a24822 #286

Merged

nobiit pushed a commit to nobidev/docker-cli that referenced this pull request Nov 19, 2025

Merge pull request docker#236 from thaJeztah/17.09-fix-secrets-and-co…

508a263

…nfigs-idempotence [17.09] sort secrets and configs to ensure idempotence

Conversation

vdemeester commented Jun 23, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thaJeztah commented Jun 23, 2017

Uh oh!

thaJeztah left a comment

Choose a reason for hiding this comment

Uh oh!

thaJeztah Jun 23, 2017

Choose a reason for hiding this comment

Uh oh!

dnephin Jun 23, 2017

Choose a reason for hiding this comment

Uh oh!

dnephin Jun 23, 2017

Choose a reason for hiding this comment

Uh oh!

vdemeester Jun 23, 2017

Choose a reason for hiding this comment

Uh oh!

vdemeester commented Jun 23, 2017

Uh oh!

codecov-io commented Jun 23, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

dnephin Jun 23, 2017

Choose a reason for hiding this comment

Uh oh!

vdemeester Jun 23, 2017

Choose a reason for hiding this comment

Uh oh!

vdemeester Jun 23, 2017

Choose a reason for hiding this comment

Uh oh!

thaJeztah commented Jun 23, 2017

Uh oh!

vdemeester commented Jun 23, 2017

Uh oh!

thaJeztah left a comment

Choose a reason for hiding this comment

Uh oh!

dnephin left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

vdemeester commented Jun 23, 2017 •

edited

Loading

codecov-io commented Jun 23, 2017 •

edited

Loading