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

[scheduler cleanup phase 2]: Rename pkg/scheduler/util to pkg/scheduler/internal/podinfo #71879

Closed
wants to merge 1 commit into from
Closed

[scheduler cleanup phase 2]: Rename pkg/scheduler/util to pkg/scheduler/internal/podinfo #71879

wants to merge 1 commit into from

Conversation

mysunshine92
Copy link
Contributor

@mysunshine92 mysunshine92 commented Dec 8, 2018 •
edited

What type of PR is this?

Uncomment only one, leave it on its own line:

/kind cleanup

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #71863

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

NONE

@k8s-ci-robot k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Dec 8, 2018
@k8s-ci-robot
Copy link
Contributor

Hi @mysunshine92. Thanks for your PR.

I'm waiting for a kubernetes 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 needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Dec 8, 2018
@k8s-ci-robot k8s-ci-robot added area/kubeadm area/kubectl area/kubelet sig/apps Categorizes an issue or PR as relevant to SIG Apps. sig/cli Categorizes an issue or PR as relevant to SIG CLI. sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. sig/node Categorizes an issue or PR as relevant to SIG Node. sig/scheduling Categorizes an issue or PR as relevant to SIG Scheduling. sig/testing Categorizes an issue or PR as relevant to SIG Testing. and removed needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Dec 8, 2018
@mysunshine92
Copy link
Contributor Author

mysunshine92 commented Dec 8, 2018 •
edited

/assign @deads2k @derekwaynecarr @timothysc

@k8s-ci-robot
Copy link
Contributor

@mysunshine92: GitHub didn't allow me to assign the following users: timothys.

Note that only kubernetes members and repo collaborators can be assigned.
For more information please see the contributor guide

In response to this:

/assign @deads2k @derekwaynecarr @TimothyS

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.

@wgliang
Copy link
Contributor

wgliang commented Dec 8, 2018

/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 Dec 8, 2018
@mysunshine92
Copy link
Contributor Author

/retest

@mysunshine92
Copy link
Contributor Author

/retest

@mysunshine92 mysunshine92 force-pushed the Rename-scheduler branch 3 times, most recently from 2313b2b to fd7ee76 Compare January 22, 2019 12:21
@mysunshine92
Copy link
Contributor Author

mysunshine92 commented Jan 23, 2019 •
edited

From my view, this PR still needs at least,

  1. heap.go moves into a new internal/heap package.
  2. kubelet code changes moved into a different git commit than the rest of the changes.

thank you for your help for this PR @misterikkit ,i receive your email but we cannot send email out from our company email.and for heap.go moves into a new internal/heap package, would you like to see a separate PR？

@misterikkit
Copy link

would you like to see a separate PR？

I would prefer it to be in this PR.

Sorry about the email difficulty. Is slack available to you, or should we coordinate here? If you need help splitting one git commit into two, I can help with that. (I never know people's experience level with git.)

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 26, 2019
@mysunshine92
Copy link
Contributor Author

would you like to see a separate PR？

I would prefer it to be in this PR.

Sorry about the email difficulty. Is slack available to you, or should we coordinate here? If you need help splitting one git commit into two, I can help with that. (I never know people's experience level with git.)

thank you for your help, i will modify it. @misterikkit

@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 29, 2019
@mysunshine92 mysunshine92 force-pushed the Rename-scheduler branch 4 times, most recently from 6a896b9 to 98e8f73 Compare January 31, 2019 19:08
@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 2, 2019
@mysunshine92 mysunshine92 force-pushed the Rename-scheduler branch 2 times, most recently from 0b08c15 to 4cbd8e0 Compare February 15, 2019 11:46
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 15, 2019
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: mysunshine92
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approvers: deads2k, derekwaynecarr, timothysc

If they are not already assigned, you can assign the PR to them by writing /assign @deads2k @derekwaynecarr @timothysc in a comment when ready.

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
Copy link
Contributor

@mysunshine92: The following tests failed, say /retest to rerun them all:

Test name Commit Details Rerun command
pull-kubernetes-e2e-kops-aws 8a58fa8d64b9c1b68434efd4c91dcaf0c776784f link /test pull-kubernetes-e2e-kops-aws
pull-kubernetes-bazel-build 4cbd8e0 link /test pull-kubernetes-bazel-build
pull-kubernetes-bazel-test 4cbd8e0 link /test pull-kubernetes-bazel-test
pull-kubernetes-e2e-gce 4cbd8e0 link /test pull-kubernetes-e2e-gce
pull-kubernetes-e2e-gce-device-plugin-gpu 4cbd8e0 link /test pull-kubernetes-e2e-gce-device-plugin-gpu
pull-kubernetes-local-e2e-containerized 4cbd8e0 link /test pull-kubernetes-local-e2e-containerized
pull-kubernetes-e2e-gce-100-performance 4cbd8e0 link /test pull-kubernetes-e2e-gce-100-performance
pull-kubernetes-kubemark-e2e-gce-big 4cbd8e0 link /test pull-kubernetes-kubemark-e2e-gce-big
pull-kubernetes-integration 4cbd8e0 link /test pull-kubernetes-integration
pull-kubernetes-node-e2e 4cbd8e0 link /test pull-kubernetes-node-e2e
pull-kubernetes-typecheck 4cbd8e0 link /test pull-kubernetes-typecheck
pull-kubernetes-verify 4cbd8e0 link /test pull-kubernetes-verify

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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. I understand the commands that are listed here.

@k8s-ci-robot
Copy link
Contributor

@mysunshine92: PR needs rebase.

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 needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 24, 2019
@timothysc timothysc removed their assignment Mar 4, 2019
@misterikkit
Copy link

Hi @mysunshine92, do you want to try to revive this? If you don't have time, someone has volunteered to take it over.

@mysunshine92
Copy link
Contributor Author

Hi @mysunshine92, do you want to try to revive this? If you don't have time, someone has volunteered to take it over.

ok, other people can take it over.

@misterikkit
Copy link

/close
as this is being handled in a different PR now.

@k8s-ci-robot
Copy link
Contributor

@misterikkit: Closed this PR.

In response to this:

/close
as this is being handled in a different PR now.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@liggitt liggitt liggitt left review comments

@misterikkit misterikkit misterikkit requested changes

@Pingan2017 Pingan2017 Pingan2017 approved these changes

@bowei bowei Awaiting requested review from bowei

@deads2k deads2k Awaiting requested review from deads2k

Assignees

@derekwaynecarr derekwaynecarr

@deads2k deads2k

@Pingan2017 Pingan2017

Labels
area/kubeadm area/kubectl area/kubelet cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. release-note-none Denotes a PR that doesn't merit a release note. sig/apps Categorizes an issue or PR as relevant to SIG Apps. sig/cli Categorizes an issue or PR as relevant to SIG CLI. sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. sig/node Categorizes an issue or PR as relevant to SIG Node. sig/scheduling Categorizes an issue or PR as relevant to SIG Scheduling. sig/testing Categorizes an issue or PR as relevant to SIG Testing. 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.

[scheduler cleanup phase 2]: Rename pkg/scheduler/util to pkg/scheduler/internal/podinfo
9 participants
@mysunshine92 @k8s-ci-robot @wgliang @misterikkit @liggitt @Pingan2017 @timothysc @derekwaynecarr @deads2k