Move heap into its own internal package #83233

hprateek43 · 2019-09-27T09:01:25Z

What type of PR is this?

Uncomment only one /kind <> line, hit enter to put that in a new line, and remove leading whitespaces from that line:

/kind api-change
/kind bug
/kind cleanup
/kind design
/kind documentation
/kind failing-test
/kind feature
/kind flake

What this PR does / why we need it:
This PR is in continuation of PR #76990. This PR aims to move heap into its internal package as part of #71863
Which issue(s) this PR fixes:

Fixes #71863

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

NONE

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

k8s-ci-robot · 2019-09-27T09:01:34Z

Hi @hprateek43. 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.

hprateek43 · 2019-09-27T09:08:34Z

@alculquicondor Can you give an ok-to-test ? This is in continuation of work in scheduler as discussed.

draveness

/ok-to-test
/cc @alculquicondor

pkg/scheduler/internal/heap/heap.go

alculquicondor · 2019-09-27T14:13:39Z

pkg/scheduler/internal/heap/heap.go

+
+// LessFunc is a function that receives two items and returns true if the first
+// item should be placed before the second one when the list is sorted.
+type LessFunc func(item1, item2 interface{}) bool


you can make this a private alias instead. There's not much benefit on having it as defined type.

Chnages as per recommendation.

Did you try making it a private alias instead?

pkg/scheduler/util/utils.go

hprateek43 · 2019-09-27T19:48:33Z

@alculquicondor Can you review the changes?

ahg-g · 2019-10-03T18:47:48Z

What type of PR is this?

Uncomment only one /kind <> line, hit enter to put that in a new line, and remove leading whitespaces from that line:
/kind api-change
/kind bug
/kind cleanup
/kind design
/kind documentation
/kind failing-test
/kind feature
/kind flake

What this PR does / why we need it:
This PR is in continuation of PR #76990. This PR aims to move heap into its internal package as part of #71863
Which issue(s) this PR fixes:

Fixes #71863

Does this PR actually fix the issue, or is it just part of it?

hprateek43 · 2019-10-04T04:43:44Z

What type of PR is this?

Uncomment only one /kind <> line, hit enter to put that in a new line, and remove leading whitespaces from that line:
/kind api-change
/kind bug
/kind cleanup
/kind design
/kind documentation
/kind failing-test
/kind feature
/kind flake

What this PR does / why we need it:
This PR is in continuation of PR #76990. This PR aims to move heap into its internal package as part of #71863
Which issue(s) this PR fixes:
Fixes #71863

Does this PR actually fix the issue, or is it just part of it?

This PR solves a part of the issue. The previous PR was split into 3 PRs. I think I should remove the fixes part from the PR description

ahg-g · 2019-10-04T13:26:22Z

/approve
/hold

for @alculquicondor to lgtm

k8s-ci-robot · 2019-10-04T13:27:00Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ahg-g, hprateek43

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:

~~pkg/scheduler/OWNERS~~ [ahg-g]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

ahg-g · 2019-10-04T13:27:39Z

What type of PR is this?

Uncomment only one /kind <> line, hit enter to put that in a new line, and remove leading whitespaces from that line:
/kind api-change
/kind bug
/kind cleanup
/kind design
/kind documentation
/kind failing-test
/kind feature
/kind flake

What this PR does / why we need it:
This PR is in continuation of PR #76990. This PR aims to move heap into its internal package as part of #71863
Which issue(s) this PR fixes:
Fixes #71863

Does this PR actually fix the issue, or is it just part of it?

This PR solves a part of the issue. The previous PR was split into 3 PRs. I think I should remove the fixes part from the PR description

please update the first comment and replace "Fixes #71863" with "Part of #71863", otherwise this will cause the issue to close.

hprateek43 · 2019-10-04T18:11:17Z

/test pull-kubernetes-e2e-gce-100-performance

alculquicondor · 2019-10-04T18:39:34Z

You can squash now

/lgtm

hprateek43 · 2019-10-04T19:13:52Z

/test pull-kubernetes-kubemark-e2e-gce-big

alculquicondor · 2019-10-04T19:40:31Z

I think the bots are back online

/hold cancel

alculquicondor · 2019-10-04T19:45:57Z

/hold cancel

ahg-g · 2019-10-04T20:18:25Z

/lgtm

fejta-bot · 2019-10-04T23:53:09Z

/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 or /hold comment for consistent failures.

fejta-bot · 2019-10-05T03:02:11Z

/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 or /hold comment for consistent failures.

fejta-bot · 2019-10-05T05:29:09Z

/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 or /hold comment for consistent failures.

k8s-ci-robot added sig/scheduling Categorizes an issue or PR as relevant to SIG Scheduling. and removed needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Sep 27, 2019

k8s-ci-robot requested review from Huang-Wei and tallclair September 27, 2019 09:03

hprateek43 force-pushed the heapPackageMovement branch from 80e530f to 55b9d5e Compare September 27, 2019 09:08

k8s-ci-robot requested a review from alculquicondor September 27, 2019 09:59

draveness reviewed Sep 27, 2019

View reviewed changes

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 Sep 27, 2019

alculquicondor reviewed Sep 27, 2019

View reviewed changes

hprateek43 force-pushed the heapPackageMovement branch 2 times, most recently from aa9bc6a to c440011 Compare September 27, 2019 17:51

hprateek43 force-pushed the heapPackageMovement branch from c440011 to 864ada0 Compare September 27, 2019 20:01

k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Sep 27, 2019

hprateek43 force-pushed the heapPackageMovement branch from 864ada0 to 6a5feca Compare September 28, 2019 09:52

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 4, 2019

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 4, 2019

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 4, 2019

hprateek43 force-pushed the heapPackageMovement branch from 27366aa to f5bc035 Compare October 4, 2019 15:26

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 4, 2019

hprateek43 force-pushed the heapPackageMovement branch from f5bc035 to 3080143 Compare October 4, 2019 16:38

k8s-ci-robot assigned alculquicondor Oct 4, 2019

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 4, 2019

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 4, 2019

Move heap into its own internal package

f462a31

hprateek43 force-pushed the heapPackageMovement branch from 3080143 to f462a31 Compare October 4, 2019 19:54

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 4, 2019

k8s-ci-robot assigned ahg-g Oct 4, 2019

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 4, 2019

k8s-ci-robot merged commit 9522fb0 into kubernetes:master Oct 5, 2019

k8s-ci-robot added this to the v1.17 milestone Oct 5, 2019

hprateek43 mentioned this pull request Oct 22, 2019

REQUEST: New membership for hprateek43 kubernetes/org#1330

Closed

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move heap into its own internal package #83233

Move heap into its own internal package #83233

hprateek43 commented Sep 27, 2019

k8s-ci-robot commented Sep 27, 2019

hprateek43 commented Sep 27, 2019 •

edited

draveness left a comment

alculquicondor Sep 27, 2019

hprateek43 Sep 27, 2019

alculquicondor Sep 27, 2019

hprateek43 commented Sep 27, 2019

ahg-g commented Oct 3, 2019

hprateek43 commented Oct 4, 2019 •

edited

ahg-g commented Oct 4, 2019

k8s-ci-robot commented Oct 4, 2019

ahg-g commented Oct 4, 2019

hprateek43 commented Oct 4, 2019

alculquicondor commented Oct 4, 2019

hprateek43 commented Oct 4, 2019

alculquicondor commented Oct 4, 2019 •

edited

alculquicondor commented Oct 4, 2019

ahg-g commented Oct 4, 2019

fejta-bot commented Oct 4, 2019

fejta-bot commented Oct 5, 2019

fejta-bot commented Oct 5, 2019

Move heap into its own internal package #83233

Move heap into its own internal package #83233

Conversation

hprateek43 commented Sep 27, 2019

k8s-ci-robot commented Sep 27, 2019

hprateek43 commented Sep 27, 2019 • edited

draveness left a comment

Choose a reason for hiding this comment

alculquicondor Sep 27, 2019

Choose a reason for hiding this comment

hprateek43 Sep 27, 2019

Choose a reason for hiding this comment

alculquicondor Sep 27, 2019

Choose a reason for hiding this comment

hprateek43 commented Sep 27, 2019

ahg-g commented Oct 3, 2019

hprateek43 commented Oct 4, 2019 • edited

ahg-g commented Oct 4, 2019

k8s-ci-robot commented Oct 4, 2019

ahg-g commented Oct 4, 2019

hprateek43 commented Oct 4, 2019

alculquicondor commented Oct 4, 2019

hprateek43 commented Oct 4, 2019

alculquicondor commented Oct 4, 2019 • edited

alculquicondor commented Oct 4, 2019

ahg-g commented Oct 4, 2019

fejta-bot commented Oct 4, 2019

fejta-bot commented Oct 5, 2019

fejta-bot commented Oct 5, 2019

hprateek43 commented Sep 27, 2019 •

edited

hprateek43 commented Oct 4, 2019 •

edited

alculquicondor commented Oct 4, 2019 •

edited