Fix up alignment of columns w/ namespaces. #10851
Conversation
jbeda
changed the title
|
GCE e2e build/test passed for commit 3aa375e52ddb98df6755cc5afb83f00c77e18409. |
|
GCE e2e build/test failed for commit addd73cdcb546bd0baaa834d32fe8474ebb4917f. |
|
GCE e2e build/test failed for commit 578cf2906e580f473dbeb82147b0937d79a62c83. |
|
@jbeda, we are on a api/cli freeze for v1.0. I am moving this to v1.0-post. Feel free to comment and/or move it back if you think this PR should make it in v1.0. |
|
Up to you, but there is a good chance that users will be very confused by This should be a pretty safe PR.
|
I agree, but it isn't really up to me :) @bgrant0607 stated that only p0 bug fixes should go in. /cc @jlowdermilk, what do you think? |
|
I think it's worth adding, provided it goes in with an Example that tests the full expected output, along the lines of https://github.com/GoogleCloudPlatform/kubernetes/blob/master/pkg/kubectl/cmd/cmd_test.go#L244 |
|
I'll try to add a test today if I can find the time. Note that the test you pointed at doesn't test the full output -- only that things don't crash. I'll see what I can do to test that the columns line up. |
|
It actually does test full output. Golang examples compare stdout with the contents of the output comment at the bottom. Kind of a weird pattern but easy to use. |
|
@jlowdermilk I think I knew that at one point. Seriously non-intuitive though. I'll add a test that handles both label columns and "extra lines". |
|
I'm having a hard time making this work with lines with trailing whitespace. I think this is a golang issue: golang/go#6416. I'll probably write a "trailing whitespace removal" filter stream to put between the writer and stdout in the test. That'll have to wait for later though as I'm probably out of time for the day. |
|
We found a Contributor License Agreement for you (the sender of this pull request) and all commit authors, but as best as we can tell these commits were authored by someone else. If that's the case, please add them to this pull request and have them confirm that they're okay with these commits being contributed to Google. If we're mistaken and you did author these commits, just reply here to confirm. |
|
CLAs look good, thanks! |
|
@jlowdermilk PTAL -- test there now. Was more difficult than it should have been :/ |
|
GCE e2e build/test passed for commit 8cdcbeba7dfd07dc312d444b9cd9687710b3e281. |
|
GCE e2e build/test passed for commit 94c6223ae819e3daef44d7c27a1d28f96dd35536. |
|
LGTM, thanks for adding test. Sorry it ended up being such a pain. |
|
@jlowdermilk Not sure how you guys are merging these days so I'll leave it to you to merge. Don't want to step on toes. |
|
Thanks @jbeda. We're in merge freeze (excepting p0 bugfixes) until 1.0 is cut, so this will probably get merged next week. |
Fixes kubernetes#10842 All issues for types that use "extra lines" for extended information. Two issues fixed: (1) When namespaces are listed an extra column isn't inserted for extra lines and (2) trailing tabs aren't inserted when label columns are specified. This code should probably move to a more explicit model of putting data into "cells". The test for this hits golang/go#6416 and so I introduced a "LineDelimiter" writer filter to make white space more visible.
|
GCE e2e build/test passed for commit 0426ab3. |
|
See also #10936 |
j3ffml
added
the
lgtm
|
With @bgrant0607's permission, adding ok-to-merge. Risk: low. This fixes misaligned kubectl output and includes unit test to catch regressions. |
Fix up alignment of columns w/ namespaces.
|
Fixes #10818. |
|
This broke shippable as it introduces syntax that isn't valid for Go 1.3. |
Fix up #10851 to be golang 1.3 compatible.
Fixes #10842
All issues for types that use "extra lines" for extended information. Two issues fixed: (1) When namespaces are listed an extra column isn't inserted for extra lines and (2) trailing tabs aren't inserted when label columns are specified.
This code should probably move to a more explicit model of putting data into "cells".
Also updated list of resources for help output for
kubectl get.