PingCAP-QE / ci

Continue intergration tests
Apache License 2.0
19 stars 96 forks source link

feat(prow-jobs/tikv/raft-engine): add prow job #2985

Open wuhuizuo opened 3 weeks ago

wuhuizuo commented 3 weeks ago

Close #2935

ti-chi-bot[bot] commented 3 weeks ago

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, it seems that this pull request adds a Prow job for TikV's Raft Engine, which will run Rust tests on two different toolchains: Rust stable and Rust nightly.

The changes added in the pull request are contained in presubmits.yaml file. The file is being added as a new file and it contains the configurations for the Prow job.

From the diff, it seems that the Prow job is well-defined and properly configured. The job is set to run the Rust tests on both Rust stable and Rust nightly toolchains, and it also includes the necessary commands to install and configure the toolchains.

However, there are a couple of suggestions for the pull request:

Overall, the changes seem good and the Prow job is well-configured. Therefore, the pull request can be approved after the suggested changes have been made.

wuhuizuo commented 1 week ago

/hold

ti-chi-bot[bot] commented 1 week ago

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

Based on the provided information, the pull request adds a new Prow job to the tikv/raft-engine repository. The job runs two tests, one using the Rust nightly channel and another using the stable channel.

There are no obvious potential problems with this pull request, but it would be best to confirm that the new job runs as expected before merging it. Additionally, it may be helpful to add more information to the pull request description, such as why this job is being added and what it is testing.

As for fixing suggestions, it would be great to add more details about the tests being run in the job and any dependencies required. Also, it might be useful to add a comment in the kustomization.yaml file indicating where the new tikv/raft-engine job is located in the list of jobs for easier reference.

ti-chi-bot[bot] commented 1 week ago

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Once this PR has been reviewed and has the lgtm label, please ask for approval from wuhuizuo, ensuring that each of them provides their approval before proceeding. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files: - **[OWNERS](https://github.com/PingCAP-QE/ci/blob/main/OWNERS)** 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[bot] commented 1 week ago

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, it appears that the changeset adds a new Prow job for the tikv/raft-engine repository. The changeset modifies the prow-jobs/kustomization.yaml file by adding a new entry for the tikv/raft-engine Prow job, and it also adds a new presubmits.yaml file for the tikv/raft-engine repository.

The changeset looks good overall, as it only adds new Prow jobs and does not modify any existing ones. However, there is one potential problem with the changeset. In the update-prow-job-kustomization.sh script, the --exit-code flag has been removed from the git diff command. This means that the script will no longer exit with a non-zero status code if the prow-jobs/kustomization.yaml file is not up to date, which could lead to issues in the future if changes are made to the Prow jobs and the kustomization file is not updated accordingly.

To fix this issue, I suggest adding the --exit-code flag back to the git diff command in the update-prow-job-kustomization.sh script. This will ensure that the script exits with a non-zero status code if the kustomization file is not up to date.

Other than this issue, the changeset looks good to me.

ti-chi-bot[bot] commented 1 week ago

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

Summary:

This pull request adds a new Prow job for the tikv/raft-engine repository. It modifies the prow-jobs/kustomization.yaml file to include the new job, and adds the new job's configuration file prow-jobs/tikv/raft-engine/presubmits.yaml.

Potential problems:

There are no potential problems with this pull request.

Fixing suggestions:

No fixing suggestions required.

Additional comments:

The changes look good and are well documented. It's always good practice to include a description of the changes that have been made in the pull request description, which is done here.