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(pingcap/ticdc): we build and publish cdc components from pingcap/ticdc repo since v9.0.0 #529

Merged
merged 2 commits into from
Jan 9, 2025

Conversation

wuhuizuo
Copy link
Contributor

@wuhuizuo wuhuizuo commented Jan 9, 2025

This pull request includes changes to the Dockerfile for the ticdc product and updates to the packages.yaml.tmpl file to modify the build and publish process for CDC components. The most important changes are:

Dockerfile changes:

  • Added a new base image argument and updated the Dockerfile to use this base image for building the ticdc product.

Build and publish process updates:

  • Updated the version constraint and added new steps to build and publish CDC components from the pingcap/ticdc repository for versions starting from v9.0.0.
  • Added detailed steps for installing the Node.js toolchain and building dm-master, dm-worker, dmctl, and dm-syncer components.
  • Defined new artifact configurations for dm-master, dm-worker, dmctl, and container images for dm and tiflow-engine since v9.0.0

@ti-chi-bot ti-chi-bot bot requested a review from purelind January 9, 2025 11:00
Copy link

ti-chi-bot bot commented Jan 9, 2025

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

Based on the PR title and the diff, it seems that the pull request is adding a new router block to the packages.yaml.tmpl file. This new block is added for the range [v9.0.0,) and builds and publishes CDC components from pingcap/ticdc repository.

There are no potential problems found in the diff.

However, I have some suggestions to improve the PR:

  1. Add a brief description in the PR description about what this PR does.
  2. Explain why the changes are necessary and what benefits they will bring.
  3. Add some examples in the PR description about how to use the new router block and what changes people need to make in their configuration files.

Overall, the changes seem to be well written and there are no potential issues found.

@ti-chi-bot ti-chi-bot bot added the size/L label Jan 9, 2025
Copy link

ti-chi-bot bot commented Jan 9, 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 diff, it seems that the changes aim to build and publish CDC components from pingcap/ticdc repo since version 9.0.0. The pull request adds a Dockerfile for CDC, modifies the package.yaml.tmpl file to build and publish CDC components, and removes the CDC builder image from the list of components to build and publish.

There are no potential problems in the pull request. However, some suggestions to improve the pull request could be:

  • Provide a more detailed description of the changes made in the pull request.
  • Add some context on why the changes were made and what problems they aim to solve.
  • Consider adding some tests to ensure that the changes do not break any existing functionality.

Overall, the pull request seems to be well-written and well-organized.

Copy link

ti-chi-bot bot commented Jan 9, 2025

@wlwilliamx: 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.

@wuhuizuo
Copy link
Contributor Author

wuhuizuo commented Jan 9, 2025

/approve

Copy link

ti-chi-bot bot commented Jan 9, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: wlwilliamx, 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 Jan 9, 2025
@ti-chi-bot ti-chi-bot bot merged commit ee7bfd3 into main Jan 9, 2025
3 checks passed
@ti-chi-bot ti-chi-bot bot deleted the fix/build-cdc-from-ticdc branch January 9, 2025 12:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants