Skip to content

Conversation

@yuxiqian
Copy link
Member

This closes FLINK-37104.

...by refactoring transform module to improve code readability and maintainability. No behavior changes are expected for all valid use cases.

Kindly ping @aiwenmo as he is the main developer of flink-runtime package.

@aiwenmo
Copy link
Contributor

aiwenmo commented Apr 10, 2025

Hi. thx @yuxiqian . I will TAL.

Copy link
Contributor

@aiwenmo aiwenmo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. The logic of the refactored code is much clearer. All the tests have passed, and currently, I can't come up with better improvement suggestions. What I want to confirm is whether this PR should include the code improvement for Batch.

@yuxiqian
Copy link
Member Author

@aiwenmo I'm okay with merging #3812 first and fix this PR later, since your PR has been opened for a long time with huge amount of changes.

@leonardBang
Copy link
Contributor

@aiwenmo I'm okay with merging #3812 first and fix this PR later, since your PR has been opened for a long time with huge amount of changes.

@yuxiqian Could you rebase this PR as #3812 has been merged?

@yuxiqian
Copy link
Member Author

Done rebasing & resolving conflicts. I removed batch-version PreTransformOpetator since it's basically the same as the streaming one. Could @aiwenmo please double check it?

Copy link
Contributor

@leonardBang leonardBang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @yuxiqian for the contribution and @aiwenmo for the review work, LGTM

@leonardBang leonardBang merged commit 4bc7366 into apache:master Apr 23, 2025
26 of 27 checks passed
linjianchang pushed a commit to linjianchang/flink-cdc that referenced this pull request May 16, 2025
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.

3 participants