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

refactor(pingcap/tidb-binlog): refactor integration-test job to prow style #3372

Merged
merged 1 commit into from
Feb 6, 2025

Conversation

wuhuizuo
Copy link
Collaborator

@wuhuizuo wuhuizuo commented Feb 6, 2025

Close #3342

@ti-chi-bot ti-chi-bot bot requested a review from purelind February 6, 2025 15:57
Copy link

ti-chi-bot bot commented Feb 6, 2025

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

Based on the diff, the key changes made in this pull request are:

  • The image in the build container has been changed from ghcr.io/pingcap-qe/ci/base:v2024.10.8-81-gec616ff-go1.21 to hub.pingcap.net/jenkins/centos7_golang-1.21:latest.
  • A new job wip-pull-integration-test has been added with a new spec and containers.

As for potential problems, there are a few things to consider:

  • It's not clear why the image in the build container has been changed, and whether this change will have any impact on the CI/CD pipeline.
  • The wip-pull-integration-test job is marked as optional, which means it may not always run. This could lead to potential issues if there are changes made to the codebase that aren't properly tested.
  • The wip-pull-integration-test job has a few environment variables that are hard-coded (e.g. KAFKA_ADDRS=127.0.0.1:9092). This could cause issues if these values need to be changed in the future.

To fix these issues, I would suggest the following:

  • The reason for changing the image in the build container should be documented in the pull request description, and any potential impact on the pipeline should be evaluated.
  • The wip-pull-integration-test job should not be marked as optional, as it's important to ensure that all changes to the codebase are properly tested.
  • The hard-coded environment variables in the wip-pull-integration-test job should be parameterized, so that they can be easily changed in the future if needed.

@ti-chi-bot ti-chi-bot bot added the size/M label Feb 6, 2025
@wuhuizuo wuhuizuo added the lgtm label Feb 6, 2025
Copy link

ti-chi-bot bot commented Feb 6, 2025

[LGTM Timeline notifier]

Timeline:

  • 2025-02-06 15:59:54.655836839 +0000 UTC m=+373866.188248837: ✖️🔁 reset by wuhuizuo.

@ti-chi-bot ti-chi-bot bot removed the lgtm label Feb 6, 2025
Copy link

ti-chi-bot bot commented Feb 6, 2025

New changes are detected. LGTM label has been removed.

@wuhuizuo wuhuizuo added the lgtm label Feb 6, 2025
@wuhuizuo
Copy link
Collaborator Author

wuhuizuo commented Feb 6, 2025

/approve

Copy link

ti-chi-bot bot commented Feb 6, 2025

[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

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 Feb 6, 2025
@ti-chi-bot ti-chi-bot bot merged commit 7fbd5a4 into main Feb 6, 2025
2 checks passed
@ti-chi-bot ti-chi-bot bot deleted the wuhuizuo/issue3342 branch February 6, 2025 16:03
wuhuizuo added a commit that referenced this pull request Feb 7, 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.

deprecate job https://ci2.pingcap.net/job/binlog_ghpr_integration/
1 participant