ci
ci copied to clipboard
feat(tiflash): install clang-format-17 if not exist
I have already done a preliminary review for you, and I hope to help you do a better job.
This pull request seems to be about changing the version of clang-format used in the tiflash pipeline from version 12 or 15 to version 17. The pull request also updates the docker image used in the pipeline.
Key changes:
-
The docker image was changed from
hub.pingcap.net/tiflash/tiflash-llvm13-amd64:v20231214
tohub.pingcap.net/tiflash/tiflash-llvm-base:amd64-llvm-17.0.6
in several files. -
The
Prepare tools
stage that was installing several tools like ccache, cmake3, clang-format, clang-format-15, clang-tidy, and gcovr, was completely removed from the pipeline in several groovy files.
Potential problems:
-
The pull request is missing a detailed description. It would be helpful to provide a rationale for these changes.
-
By removing the
Prepare tools
stage, the pipeline now assumes that all required tools are pre-installed in the new docker image. If this is not the case, the pipeline could fail. -
The change in clang-format version may have implications on the codebase if the new version introduces any breaking changes or behaves differently from the previous version.
Fixing suggestions:
-
Add a detailed description explaining the reasons for the changes, the impact, and the benefits.
-
Ensure that all the tools previously installed in the
Prepare tools
stage are indeed present in the new Docker image. -
Test the pipeline with the new clang-format version to make sure it behaves as expected.
/hold
[LGTM Timeline notifier]
Timeline:
-
2024-06-25 06:32:29.121395787 +0000 UTC m=+701275.606884621
: :ballot_box_with_check: agreed by purelind.
/cc @JaySon-Huang
@JaySon-Huang: adding LGTM is restricted to approvers and reviewers in OWNERS files.
In response to this:
LGTM
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.
[APPROVALNOTIFIER] This PR is APPROVED
This pull-request has been approved by: JaySon-Huang, purelind
The full list of commands accepted by this bot can be found here.
The pull request process is described here
- ~~pipelines/OWNERS~~ [purelind]
Approvers can indicate their approval by writing /approve
in a comment
Approvers can cancel approval by writing /approve cancel
in a comment
/unhold