X Tutup
Skip to content

Fix docker version output alignment#965

Merged
vdemeester merged 1 commit intodocker:masterfrom
thaJeztah:fix-version-output
Mar 28, 2018
Merged

Fix docker version output alignment#965
vdemeester merged 1 commit intodocker:masterfrom
thaJeztah:fix-version-output

Conversation

@thaJeztah
Copy link
Member

@thaJeztah thaJeztah commented Mar 26, 2018

closes #964
closes #769
closes #913

Use tabwriter to print the version output

@tiborvass I tried using a tab writer, and this seems to work correctly; do you know what bug you ran into?

Output with this change:

Client:
 Version:      18.04.0-dev
 API version:  1.37
 Go version:   go1.9.4
 Git commit:   76439457
 Built:        Mon Mar 26 14:10:50 2018
 OS/Arch:      linux/amd64
 Experimental: false
 Orchestrator: swarm

Server:
 Engine:
  Version:      18.03.0-ce-rc4
  API version:  1.37 (minimum version 1.12)
  Go version:   go1.9.4
  Git commit:   fbedb97
  Built:        Thu Mar 15 07:42:29 2018
  OS/Arch:      linux/amd64
  Experimental: true

Copy link
Collaborator

@vdemeester vdemeester left a comment

Choose a reason for hiding this comment

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

LGTM 🐫

@thaJeztah
Copy link
Member Author

Or do we want "client" and "server" columns to be aligned? (guess we can do so by making the "min" width bigger)

err = err2
}
dockerCli.Out().Write([]byte{'\n'})
buffer.WriteTo(t)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do you need a buffer here? Can't you use t as the first arg to tmpl.Execute ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh! was debugging, looks like I don't need it here anymore no.

Use tabwriter to print the version output

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah thaJeztah force-pushed the fix-version-output branch from 69003e9 to 48eb7a0 Compare March 26, 2018 15:59
Copy link
Contributor

@dnephin dnephin left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@seemethere seemethere left a comment

Choose a reason for hiding this comment

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

LGTM

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

X Tutup