Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(helm): Use line breaks consistently in status output #4809

Merged
merged 1 commit into from
Nov 9, 2018
Merged

fix(helm): Use line breaks consistently in status output #4809

merged 1 commit into from
Nov 9, 2018

Conversation

mortent
Copy link

@mortent mortent commented Oct 19, 2018

The output from helm status does not have consistent use of line breaks.
For some resources there is a line break after the kind header, for
others there is not. This is caused by how the printer handles column
headers. This removes a line break for all but the first resource listed.

Signed-off-by: Morten Torkildsen mortent@google.com

The output from helm status does not have consistent use of line breaks.
For some resources there is a line break after the kind header, for
others there is not. This is caused by how the printer handles column
headers. This removes a line break for all but the first resource listed.

Signed-off-by: Morten Torkildsen <mortent@google.com>
@bacongobbler bacongobbler added this to the 2.12.0 - Features milestone Nov 6, 2018
@mattfarina
Copy link
Collaborator

screen shot 2018-11-09 at 10 24 53 am

On the left is before the PR and on the right is after the PR.

@mattfarina
Copy link
Collaborator

For reference, this has to do with how Kubernetes deals with printing human readable material. Specifically, see the human readable printer at https://github.com/kubernetes/kubernetes/blob/17c77c7898218073f14c8d573582e8d2313dc740/pkg/printers/humanreadable.go#L308-L310

		if h.lastType != nil && h.lastType != t && !h.options.NoHeaders {
			fmt.Fprintln(output)
		}

If there was a previous type printed it adds the new line. That's why we need it on the first one but not the rest.

@mattfarina mattfarina merged commit e91e961 into helm:master Nov 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants