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

feat: switch to use new pipeline trigger by prow in branch 7.5 #3135

Conversation

purelind
Copy link
Collaborator

Swtich to use new pipeline trigger by prow in tikv & tiflash branch release-7.5

@ti-chi-bot ti-chi-bot bot requested a review from wuhuizuo September 24, 2024 07:02
Copy link

ti-chi-bot bot commented Sep 24, 2024

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

Based on the pull request title and description, the key changes are that this pull request switches to using a new pipeline trigger by prow in the release-7.5 branch for the tikv and tiflash repositories.

Looking at the diff, the changes made are mainly in the Jenkins pipeline configuration files for the tikv and tiflash repositories and in the config.py file for the tidb-dashboard repository. The changes add release-7.5 as a blacklisted target branch in the Jenkins pipeline configuration files, indicating that pull requests targeting that branch should not trigger the pipeline. The changes to config.py for the tidb-dashboard repository have been commented out, which seems to indicate that the changes are not yet complete.

One potential problem with this pull request is that there is no explanation provided for why the new pipeline trigger is being used or what benefits it brings. It would be helpful to have more context on why this change is being made and what impact it will have on the CI/CD process.

Another potential issue is the change made to config.py for the tidb-dashboard repository. The changes have been commented out, which could indicate that they are not yet complete or that they are being temporarily disabled. It is not clear what the intention is here, so more information is needed.

Overall, the changes made in this pull request seem to be relatively minor and straightforward. To improve the pull request, it would be helpful to provide more context on why the new pipeline trigger is being used and to clarify the intentions behind the changes to config.py for the tidb-dashboard repository.

@ti-chi-bot ti-chi-bot bot added the size/M label Sep 24, 2024
Copy link

ti-chi-bot bot commented Sep 24, 2024

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

Based on the pull request, the change is to switch to a new pipeline trigger by prow in tikv & tiflash branch release-7.5. The diff shows the changes in the blackListTargetBranches for the ghprbBranch.

There are no obvious potential problems with this pull request, but it's important to ensure that the new pipeline trigger is fully tested and validated before merging.

One suggestion for improvement would be to update the pull request description with more details about the new pipeline trigger and any testing that has been done to validate it. This would provide more context for reviewers and make it easier to assess the impact of the change.

@ti-chi-bot ti-chi-bot bot added size/XS and removed size/M labels Sep 24, 2024
Copy link

ti-chi-bot bot commented Sep 24, 2024

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

Based on the PR title and description, the changes seem to be related to switching to a new pipeline trigger for the tikv and tiflash branches for release-7.5. Here are some potential issues and suggestions for fixing them:

  1. In the tikv job, the blackListTargetBranches section is missing the release-7.5 branch, which means that it is not being excluded from the job. The same issue exists in the tiflash jobs. The suggested fix is to add ghprbBranch { branch('release-7.5') } to the blackListTargetBranches section for both jobs.

  2. In the tikv job, the always_run, optional, and skip_report options are set to false, which means that the job will not run by default. The suggested fix is to set these options to true so that the job runs.

  3. In the tikv job, the context field is set to wip/pull-unit-test, which seems incorrect. The suggested fix is to change it to pull-unit-test.

  4. In the tikv job, the trigger and rerun_command fields seem to be incorrect. The suggested fix is to change them to "(?m)^/test (?:.*? )?pull-unit-test(?: .*?)?$" and "/test pull-unit-test", respectively.

Overall, the changes seem reasonable, but the issues identified above should be fixed before merging the PR.

@ti-chi-bot ti-chi-bot bot added size/S and removed size/XS labels Sep 24, 2024
Copy link

ti-chi-bot bot commented Sep 24, 2024

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

Key changes:

  • This pull request updates the pipeline trigger in the release-7.5 branch for tikv and tiflash.
  • It updates the blackListTargetBranches to prevent triggering pipelines in master and other release branches except release-7.5.
  • It updates the prow-jobs configuration files for tiflash and tikv to use the new pipeline trigger in the release-7.5 branch.

Potential problems:

  • It is not clear why the pipeline trigger needs to be changed specifically for the release-7.5 branch. This should be explained in the pull request description or in the commit messages.
  • It is not clear if any testing or validation has been done on the new pipeline trigger before this pull request was submitted.

Fixing suggestions:

  • The pull request description should be updated to clarify the reason for the pipeline trigger change.
  • The commit messages should provide more details about the changes made and any testing or validation done.
  • It would be helpful to add some testing or validation steps in the pull request description or in the Jenkinsfile to ensure that the new pipeline trigger is working as expected.

@ti-chi-bot ti-chi-bot bot added size/M and removed size/S labels Sep 24, 2024
@purelind
Copy link
Collaborator Author

/hold

@purelind
Copy link
Collaborator Author

tiflash

image image ## tikv image

@purelind
Copy link
Collaborator Author

Copy link

ti-chi-bot bot commented Sep 25, 2024

@purelind: GitHub didn't allow me to request PR reviews from the following users: zhangjinpeng1987.

Note that only PingCAP-QE members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

/cc @Lloyd-Pottiger @JaySon-Huang @zanmato1984 @glorv @zhangjinpeng1987

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.

Copy link

ti-chi-bot bot commented Sep 25, 2024

@glorv: adding LGTM is restricted to approvers and reviewers in OWNERS files.

In response to this:

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.

Copy link

ti-chi-bot bot commented Sep 25, 2024

@Lloyd-Pottiger: adding LGTM is restricted to approvers and reviewers in OWNERS files.

In response to this:

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.

Copy link

ti-chi-bot bot commented Sep 25, 2024

@zanmato1984: adding LGTM is restricted to approvers and reviewers in OWNERS files.

In response to this:

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.

@wuhuizuo
Copy link
Collaborator

/approve

Copy link

ti-chi-bot bot commented Sep 25, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: glorv, Lloyd-Pottiger, wuhuizuo, zanmato1984

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 Sep 25, 2024
@purelind
Copy link
Collaborator Author

/unhold

@purelind purelind added the lgtm label Sep 25, 2024
@ti-chi-bot ti-chi-bot bot merged commit 645cc7b into PingCAP-QE:main Sep 25, 2024
2 checks passed
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.

5 participants