-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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 regression in ps with custom format #2232
Conversation
Firstly... ew. This code is useful, certainly (for formatting output for scripts and such), but if I could I'd exorcise this particular demon and pretend it never existed. Secondly, LGTM. |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: baude, mheon The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Go formatting is ... ew. |
@baude you make Go Formatting sound better than it is imo. Ugh. |
LGTM, assuming happy tests. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
return err | ||
} | ||
|
||
fmt.Fprintln(w, "") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fmt.Fprintf(w, "\n") Seems clearer as to intention.
Oh, wow! Thanks @baude We happened to have just noticed this same regression when hacking something up for the Fedora Toolbox. |
test/e2e/ps_test.go
Outdated
Expect(result.ExitCode()).To(Equal(0)) | ||
Expect(result.IsJSONOutputValid()).To(BeTrue()) | ||
Expect(result.IsJSONOutputValid()).To(BeFalse()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We want invalid json output?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Im not sure what the history is there but the output should NOT be json. maybe it should just be removed.
Using the table keyword in go templating had regressed and was no longer working. Fixes: 2221 Signed-off-by: baude <[email protected]>
/lgtm |
Using the table keyword in go templating had regressed and was
no longer working.
Fixes: 2221
Signed-off-by: baude [email protected]