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

Migrate RemovePodsViolatingNodeAffinity to plugin #860

Merged
merged 1 commit into from
Aug 9, 2022
Merged

Migrate RemovePodsViolatingNodeAffinity to plugin #860

merged 1 commit into from
Aug 9, 2022

Conversation

knelasevero
Copy link
Contributor

Built on top of #857
/hold
until #846 and #857 (both) get merged

I will be rebasing review changes from those into here.

@k8s-ci-robot k8s-ci-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jun 23, 2022
@k8s-ci-robot
Copy link
Contributor

Hi @knelasevero. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@k8s-ci-robot k8s-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Jun 23, 2022
@knelasevero knelasevero mentioned this pull request Jun 23, 2022
Closed
12 tasks
@a7i
Copy link
Contributor

a7i commented Jun 24, 2022

/ok-to-test

@k8s-ci-robot k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jun 24, 2022
@knelasevero
Copy link
Contributor Author

🤔 hmm, so the new case in the switch/case is the culprit of the cyclomatic complexity overshoot here 😢? I will try to take care of it later

@knelasevero
Copy link
Contributor Author

TBH probably best if @ingvagabund reworks that method to reduce cyclomatic complexity on his PR so we all get those changes to work on top of it. Then we don´t have to cherry pick stuff? WDYT?

@a7i
Copy link
Contributor

a7i commented Jun 24, 2022

TBH probably best if @ingvagabund reworks that method to reduce cyclomatic complexity on his PR so we all get those changes to work on top of it. Then we don´t have to cherry pick stuff? WDYT?

I agree. I'm also having the same issue and it's going to be some nasty merge conflicts if we all did it.

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 21, 2022
This was referenced Jul 25, 2022
Merged
Closed
@knelasevero knelasevero force-pushed the migrate-node-afinity-to-plugin branch from 3fbc1c9 to 99a8dae Compare August 1, 2022 16:03
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 1, 2022
@knelasevero knelasevero force-pushed the migrate-node-afinity-to-plugin branch from 99a8dae to 81b50e5 Compare August 1, 2022 16:04
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Aug 1, 2022
@knelasevero knelasevero force-pushed the migrate-node-afinity-to-plugin branch 3 times, most recently from 0b5f50f to fb789fc Compare August 1, 2022 16:23
@k8s-ci-robot k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Aug 1, 2022
@knelasevero knelasevero force-pushed the migrate-node-afinity-to-plugin branch from fb789fc to 726b63f Compare August 1, 2022 17:22
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Aug 1, 2022
@knelasevero
Copy link
Contributor Author

/retest

@knelasevero
Copy link
Contributor Author

/unhold

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 1, 2022
@knelasevero knelasevero force-pushed the migrate-node-afinity-to-plugin branch from 726b63f to 5ce7872 Compare August 2, 2022 18:39
@knelasevero knelasevero mentioned this pull request Aug 3, 2022
Closed
9 tasks
@ingvagabund ingvagabund mentioned this pull request Aug 4, 2022
Closed
@knelasevero knelasevero force-pushed the migrate-node-afinity-to-plugin branch 3 times, most recently from a8911fe to 0d125cb Compare August 4, 2022 17:05
@knelasevero knelasevero force-pushed the migrate-node-afinity-to-plugin branch 2 times, most recently from 9615a72 to 06bd6c8 Compare August 8, 2022 15:45
@knelasevero knelasevero mentioned this pull request Aug 8, 2022
Merged
@knelasevero knelasevero force-pushed the migrate-node-afinity-to-plugin branch from 06bd6c8 to 58da66c Compare August 9, 2022 12:04
@knelasevero knelasevero force-pushed the migrate-node-afinity-to-plugin branch from 58da66c to 0c3bf7f Compare August 9, 2022 12:05
@ingvagabund
Copy link
Contributor

/approve
/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 9, 2022
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ingvagabund

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 Aug 9, 2022
@k8s-ci-robot k8s-ci-robot merged commit 788e9f8 into kubernetes-sigs:master Aug 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@a7i a7i a7i left review comments

@ingvagabund ingvagabund ingvagabund requested changes

@aveshagarwal aveshagarwal Awaiting requested review from aveshagarwal

@lixiang233 lixiang233 Awaiting requested review from lixiang233

Assignees

@ingvagabund ingvagabund

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. lgtm "Looks good to me", indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@knelasevero @k8s-ci-robot @a7i @ingvagabund