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

♻️ refactor buildPod in unit test #2982

Closed
wants to merge 2 commits into from
Closed

♻️ refactor buildPod in unit test #2982

wants to merge 2 commits into from

Conversation

kingeasternsun
Copy link
Contributor

on the base of #2901

I found that the function BuildPod, BuildPodWithPVC, BuildBestEffortPod, BuildPodWithPriority have many dumplicate code, which could be refactor use Option code model.

NOTE: #2901 should be merged firstly.

@kingeasternsun
rewrite all action tests with common CreateCacheForTest and TestArg
4817c8e 
Signed-off-by: kingeasternsun <kingeasternsun@gmail.com>
@volcano-sh-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please assign shinytang6
You can assign the PR to them by writing /assign @shinytang6 in a comment when ready.

The full list of commands accepted by this bot can be found 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

@volcano-sh-bot volcano-sh-bot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Jul 19, 2023
@kingeasternsun
rewrite build pod
33f2db3 
Signed-off-by: kingeasternsun <kingeasternsun@gmail.com>
@lowang-bh
Copy link
Member

/hold

@volcano-sh-bot volcano-sh-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 19, 2023
@stale
Copy link

stale bot commented Sep 17, 2023

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale stale bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Sep 17, 2023
@kingeasternsun
Copy link
Contributor Author

what should do to move it forward

@stale stale bot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Sep 21, 2023
@kingeasternsun
Copy link
Contributor Author

still need

1 similar comment
@kingeasternsun
Copy link
Contributor Author

still need

@kingeasternsun kingeasternsun closed this by deleting the head repository Dec 6, 2023
@kingeasternsun kingeasternsun mentioned this pull request Dec 7, 2023
Open
@kingeasternsun
Copy link
Contributor Author

Because the origin PR has two much different and conflicts with the latest master, So I decided to reopen a new PR #3251

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

@merryzhou merryzhou Awaiting requested review from merryzhou

@shinytang6 shinytang6 Awaiting requested review from shinytang6

Assignees
No one assigned
Labels
do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@kingeasternsun @volcano-sh-bot @lowang-bh