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(br): enable condition trigger of br test on 8.1 #3332

Conversation

purelind
Copy link
Collaborator

Enable condition trigger for br integration test pipepline on lts branch release-8.1,PRs that modify relevant files need to pass this test before they can be merged.

Copy link

ti-chi-bot bot commented Jan 24, 2025

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 enables a condition trigger for br integration test pipeline on the lts branch release-8.1. This means that pull requests that modify relevant files will need to pass this test before they can be merged.

The changes in the diff appear to modify the release-8.1-presubmits.yaml file, adding a run_if_changed field that specifies the files which will trigger the pipeline and removing the always_run, optional, and skip_report fields.

One potential problem is that the run_if_changed field may not capture all the relevant files that need to trigger the pipeline. It would be useful to test this by making a small change to a file that should trigger the pipeline and ensuring that the pipeline is indeed triggered.

Another potential problem is that the decorate field has been set to false, which could impact the functionality of the pipeline. It would be useful to confirm that this was a deliberate decision and won't cause any issues.

Fixing suggestions:

  • Confirm that the run_if_changed field captures all relevant files that should trigger the pipeline.
  • Confirm that setting decorate to false is a deliberate decision and won't cause any issues. If it will cause issues, suggest setting it to true.
  • Consider adding more detailed documentation to the pull request description to explain the reasoning behind these changes and how they will impact the pipeline.

@ti-chi-bot ti-chi-bot bot requested a review from wuhuizuo January 24, 2025 08:16
@ti-chi-bot ti-chi-bot bot added the size/XS label Jan 24, 2025
@purelind
Copy link
Collaborator Author

/cc @BornChanger

@ti-chi-bot ti-chi-bot bot requested a review from BornChanger January 24, 2025 08:16
@purelind purelind changed the title feat(br): enable condition trigger of br test on 8.5 feat(br): enable condition trigger of br test on 8.1 Jan 24, 2025
@purelind
Copy link
Collaborator Author

/hold
It needs to take effect when resources are relatively available.

Copy link

ti-chi-bot bot commented Jan 24, 2025

[LGTM Timeline notifier]

Timeline:

  • 2025-01-24 10:51:57.131238808 +0000 UTC m=+437244.462158225: ☑️ agreed by wuhuizuo.

Copy link

ti-chi-bot bot commented Jan 26, 2025

@BornChanger: 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-sigs/prow repository.

Copy link

ti-chi-bot bot commented Jan 26, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: BornChanger, 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

@purelind
Copy link
Collaborator Author

purelind commented Feb 5, 2025

/unhold

@ti-chi-bot ti-chi-bot bot merged commit 9bf6dea into PingCAP-QE:main Feb 5, 2025
2 checks passed
purelind added a commit that referenced this pull request Feb 5, 2025
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.

3 participants