-
Notifications
You must be signed in to change notification settings - Fork 938
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.
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
add jobflow controller code #2559
Conversation
e346b8c
to
a8bd3e9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
Generally looks good.
I think we can unify the cache in the controller later to reduce memory usage.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@zhoumingcheng hey. Can you separate the PR into some small ones for it is too large to review?
22c24b5
to
18a6b51
Compare
Thank you for your suggestion. I have broken the code into three parts: controller, e2e test, and webhook. I will give priority to submitting the code of the controller. |
1 similar comment
Thank you for your suggestion. I have broken the code into three parts: controller, e2e test, and webhook. I will give priority to submitting the code of the controller. |
18a6b51
to
bf83464
Compare
bf83464
to
b43f497
Compare
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. |
Keep current PR |
Signed-off-by: zhoumingcheng <zhoumingcheng@beyondcent.com>
@zhoumingcheng: Adding label 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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
I think we can move on to this work.
33d6496
to
2dba066
Compare
Signed-off-by: zhoumingcheng <zhoumingcheng@beyondcent.com>
2dba066
to
67e339c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
/priority important-soon |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/approve
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: william-wang 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 |
There is no jobflow crd under |
Signed-off-by: zhoumingcheng zhoumingcheng@beyondcent.com