Closed wuhuizuo closed 1 month 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 description, it seems like this PR is reverting another PR. Therefore, I would first check the original PR to understand the change that was being made and why it was reverted.
Assuming that this PR is valid, the key changes seem to be updating the URLs for Dockerfiles in several places. Specifically, the changes update the URLs for the following Dockerfiles:
jenkins/pipelines/cd/atom-jobs/docker-common.groovy
jenkins/pipelines/cd/atom-jobs/products/tiflash/master.groovy
jenkins/pipelines/cd/dev-build.groovy
jenkins/pipelines/cd/nightly/release-all-images-by-branch.groovy
jenkins/pipelines/cd/pre-release-community-docker-rocky.groovy
jenkins/pipelines/cd/pre-release-enterprise-docker-rocky.groovy
pipelines/pingcap/tidb/latest/merged_tiflash_test.groovy
pipelines/pingcap/tidb/latest/pull_tiflash_test.groovy
There don't seem to be any potential problems with these changes. However, I would suggest adding more context to the PR description to explain why this reversion is necessary. Additionally, it would be helpful to have a link to the original PR that was reverted.
As for fixing suggestions, assuming this PR is valid, there don't seem to be any necessary changes. However, as mentioned earlier, I would suggest adding more context to the PR description for better clarity.
/review The CI server say some error happened:
[2024-05-11T10:18:25.095Z] Started by upstream project "devbuild" build number 4675
[2024-05-11T10:18:25.095Z] originally caused by:
[2024-05-11T10:18:25.095Z] Started by user PingCAP System Admin
[2024-05-11T10:18:26.166Z] Obtained jenkins/pipelines/cd/atom-jobs/docker-common.groovy from git https://github.com/PingCAP-QE/ci.git
[2024-05-11T10:18:26.173Z] org.codehaus.groovy.control.MultipleCompilationErrorsException: startup failed:
[2024-05-11T10:18:26.173Z] WorkflowScript: 92: unexpected token: def @ line 92, column 3.
[2024-05-11T10:18:26.173Z] + def additionalArgs = ""
[2024-05-11T10:18:26.173Z] ^
[2024-05-11T10:18:26.173Z]
[2024-05-11T10:18:26.173Z] 1 error
[2024-05-11T10:18:26.173Z]
[2024-05-11T10:18:26.174Z] at org.codehaus.groovy.control.ErrorCollector.failIfErrors(ErrorCollector.java:309)
what's the problem?
@wuhuizuo: Based on the provided information, it seems that the error occurred in the continuous integration (CI) server during the execution of a merge request titled "refactor(jenkins/pipelines,pipelines): use the new dockerfile url location". The merge request aimed to revert a previous Pull Request from the repository with number 2962.
The changes made in this merge request involve the modification of several Dockerfile URLs and the addition of a new flag to the Docker build command. The modifications were made to the following files:
However, the error message is missing, and it is not clear what caused the issue. It is necessary to have more information to understand what went wrong and how to solve the problem.
I have already done a preliminary review for you, and I hope to help you do a better job.
Summary:
This pull request aims to refactor the Jenkins pipelines and use a new dockerfile URL location. The changes include adding additionalArgs to the buildImgagesh, removing unnecessary arguments from the Docker build command, and changing the Dockerfile URL for some pipelines.
Potential problems:
It is not clear why this PR is being reverted. It is important to understand what caused the revert and ensure the changes in this PR do not reintroduce the same issue.
Fixing suggestions:
additionalArgs
to make it clear what arguments are being added to the Docker build command.I have already done a preliminary review for you, and I hope to help you do a better job.
Key Changes:
additionalArgs
variable is added to handle optional arguments while building images.Potential Problems:
additionalArgs
variable is not being used consistently across all the buildImgagesh
definitions. It may cause confusion to users in the future.Fixing Suggestions:
additionalArgs
variable should either be used consistently across all the buildImgagesh
definitions or removed altogether if not required./approve
[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
Reverts PingCAP-QE/ci#2962