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.

Sign up for GitHub

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

Jump to bottom

fix sorting taints in case the sorting keys are equal #61255

Merged
merged 1 commit into from
Mar 25, 2018
Merged

fix sorting taints in case the sorting keys are equal #61255

merged 1 commit into from
Mar 25, 2018
+11 −15

Conversation

dixudx
Copy link
Member

@dixudx dixudx commented Mar 16, 2018

What this PR does / why we need it:
/kind bug
/sig cli

When describing node taints, the similar issue mentioned in #61250 also exists.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
xref #61250

Special notes for your reviewer:
/cc @kubernetes/sig-cli-bugs
@kubernetes/sig-cli-api-reviews

Release note:

fix sorting taints in case the sorting keys are equal

@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/cli Categorizes an issue or PR as relevant to SIG CLI. kind/bug Categorizes issue or PR as related to a bug. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Mar 16, 2018
@dixudx
Copy link
Member Author

dixudx commented Mar 16, 2018

/retest

@dixudx
Copy link
Member Author

dixudx commented Mar 19, 2018

/assign @soltysh

soltysh
soltysh approved these changes Mar 19, 2018
View reviewed changes
Copy link
Contributor

@soltysh soltysh left a comment

Choose a reason for hiding this comment

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

This lgtm, one tiny nit.

pkg/printers/internalversion/describe.go Outdated
}
sort.Strings(keys)
sort.Slice(taints, func(i, j int) bool {
return string(taints[i].Effect)+","+taints[i].Key < string(taints[j].Effect)+","+taints[j].Key
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you turn this 'stringify' method into a helper to ensure both are actually the same.

@soltysh soltysh added this to the v1.11 milestone Mar 19, 2018
soltysh
soltysh approved these changes Mar 20, 2018
View reviewed changes
Copy link
Contributor

@soltysh soltysh left a comment

Choose a reason for hiding this comment

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

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 20, 2018
@soltysh soltysh added priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. status/approved-for-milestone labels Mar 20, 2018
@dixudx
Copy link
Member Author

dixudx commented Mar 22, 2018

ping @kubernetes/sig-cli-maintainers for approval. Thanks.

@dixudx
Copy link
Member Author

dixudx commented Mar 23, 2018

/assign @liggitt

PTAL. Thanks.

@k8s-github-robot
Copy link

[MILESTONENOTIFIER] Milestone Pull Request: Up-to-date for process

@dixudx @liggitt @soltysh

Pull Request Labels
  • sig/cli: Pull Request will be escalated to these SIGs if needed.
  • priority/important-longterm: Escalate to the pull request owners; move out of the milestone after 1 attempt.
  • kind/bug: Fixes a bug discovered during the current release.
Help

@soltysh
Copy link
Contributor

soltysh commented Mar 23, 2018

/approve no-issue

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dixudx, soltysh

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 23, 2018
@fejta-bot
Copy link

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel comment for consistent failures.

@fejta-bot
Copy link

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel comment for consistent failures.

2 similar comments
@fejta-bot
Copy link

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel comment for consistent failures.

@fejta-bot
Copy link

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel comment for consistent failures.

@k8s-github-robot
Copy link

/test all [submit-queue is verifying that this PR is safe to merge]

@k8s-github-robot
Copy link

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions here.

@k8s-github-robot k8s-github-robot merged commit 83a4f27 into kubernetes:master Mar 25, 2018
@dixudx dixudx deleted the fix_describe_taint branch March 26, 2018 01:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@soltysh soltysh soltysh approved these changes

@dims dims Awaiting requested review from dims

@rootfs rootfs Awaiting requested review from rootfs

Assignees

@soltysh soltysh

@liggitt liggitt

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API kind/bug Categorizes issue or PR as related to a bug. lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/cli Categorizes an issue or PR as relevant to SIG CLI. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Milestone
v1.11
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@dixudx @k8s-github-robot @soltysh @k8s-ci-robot @fejta-bot @liggitt