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.

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

fix(ti-community-infra/prow): let the heavy job run in other cluster #2998

Merged
merged 1 commit into from
Jun 14, 2024

Conversation

wuhuizuo
Copy link
Collaborator

Signed-off-by: wuhuizuo wuhuizuo@126.com

Copy link

ti-chi-bot bot commented Jun 14, 2024

I have already done a preliminary review for you, and I hope to help you do a better job.

Based on the title and description of the pull request, it seems that the contributor is trying to move a heavy job to another cluster. Looking at the diff, the changes made are mostly related to updating the OWNERS and OWNERS_ALIASES files to add the sig-approvers-ee group as the approvers and removing the wuhuizuo, purelind, and lijie0123 reviewers.

In addition, the postsubmits.yaml file has been modified to add a cluster field to the push-prow-images job, and increase the CPU request to 4. The presubmits.yaml file has also been modified to add a clone_depth field to the pull-verify job.

Overall, the changes seem reasonable and do not appear to introduce any potential problems. However, some suggestions for improvement are:

  1. It would be helpful to provide more information about why the heavy job needs to be moved to another cluster, and what benefits it will bring.

  2. It would be good to provide some justification for the changes made to the OWNERS and OWNERS_ALIASES files, and explain why the sig-approvers-ee group has been added as the approvers.

  3. The postsubmits.yaml and presubmits.yaml files could benefit from some comments to explain the reasoning behind the changes made.

  4. It would be good to ensure that the clone_depth field is set to a reasonable value for the pull-build-images job as well, to avoid unnecessarily cloning the entire repository history.

  5. It would be a good idea to test the changes thoroughly before merging the pull request to ensure that the heavy job is running correctly on the new cluster.

Overall, the changes look good and should be merged after addressing the suggestions mentioned above.

@ti-chi-bot ti-chi-bot bot added the size/M label Jun 14, 2024
@wuhuizuo wuhuizuo added the lgtm label Jun 14, 2024
@wuhuizuo
Copy link
Collaborator Author

/approve

Copy link

ti-chi-bot bot commented Jun 14, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: wuhuizuo

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

@ti-chi-bot ti-chi-bot bot added the approved label Jun 14, 2024
@ti-chi-bot ti-chi-bot bot merged commit 5a4ceca into main Jun 14, 2024
1 of 2 checks passed
@ti-chi-bot ti-chi-bot bot deleted the fix/delete-image-build-jobs branch June 14, 2024 04:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

1 participant