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

swap FIRSTSEEN/LASTSEEN columns in kubectl get event -w #27549

Merged
merged 1 commit into from
Jun 21, 2016

Conversation

therc
Copy link
Member

@therc therc commented Jun 16, 2016

Show LASTSEEN, the sorting key, as the first column in `kubectl get event` output

Not having LASTSEEN as the first column can confuse users into thinking
that events are not delivered in order.

Fixes #27060

@therc
Copy link
Member Author

therc commented Jun 16, 2016

This is ugly, as I already mentioned. I think we should just changed the order permanently, watch or not.

cc @justinsb @dkaplan-pivotal

(More specifically, NewHumanReadablePrinter is not idempotent. If somehow it gets invoked a second time, the headers will be swapped again and thus back to the default.)

@k8s-github-robot k8s-github-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. release-note-label-needed labels Jun 16, 2016
@j3ffml
Copy link
Contributor

j3ffml commented Jun 16, 2016

Conditionally swapping is ugly. +1 for changing order permanently.

Not having LASTSEEN as the first column can confuse users into thinking
that events are not delivered in order.

Fixes kubernetes#27060
@k8s-github-robot k8s-github-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jun 16, 2016
@therc
Copy link
Member Author

therc commented Jun 16, 2016

Done.

@therc
Copy link
Member Author

therc commented Jun 16, 2016

I wouldn't complain if this change were to get merged in 1.3. It's a common WTH item.

@j3ffml j3ffml added this to the v1.3 milestone Jun 16, 2016
@j3ffml
Copy link
Contributor

j3ffml commented Jun 16, 2016

lgtm, but please edit top comment to include a one line release note in block quotes.

@therc
Copy link
Member Author

therc commented Jun 16, 2016

Done. I assumed that eliminating the block would use the subject as the release note.

@j3ffml j3ffml added release-note Denotes a PR that will be considered when it comes time to generate release notes. lgtm "Looks good to me", indicates that a PR is ready to be merged. and removed release-note-label-needed labels Jun 16, 2016
@k8s-bot
Copy link

k8s-bot commented Jun 20, 2016

GCE e2e build/test passed for commit bf873df.

@k8s-github-robot
Copy link

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

@k8s-bot
Copy link

k8s-bot commented Jun 21, 2016

GCE e2e build/test passed for commit bf873df.

@k8s-github-robot
Copy link

Automatic merge from submit-queue

@k8s-github-robot k8s-github-robot merged commit 1fd4047 into kubernetes:master Jun 21, 2016
@therc therc deleted the eventorder branch July 18, 2016 21:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants