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

[FLINK-35067][cdc-connector][postgres] Adding metadata 'row_kind' for Postgres CDC Connector. #3716

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

oli2v
Copy link

@oli2v oli2v commented Nov 11, 2024

No description provided.

@lvyanquan
Copy link
Contributor

lvyanquan commented Nov 12, 2024

Hi @oli2v, This feature is meaningful to me, but it seems that there are many irrelevant changes in the code (including test code), was this adjusted by IDE?We should try to minimize irrelevant changes as much as possible.

@oli2v
Copy link
Author

oli2v commented Nov 12, 2024

Hi @lvyanquan, those irrelevant changes in the code were indeed due to the IDE. I just pushed a new commit trying to minimize them.

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the update and it looks better, but I'm wondering this change of import orders is necessary?

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the update and it looks better, but I'm wondering this change of import orders is necessary?

You can use mvn spotless:apply to correct the import orders.

@lvyanquan
Copy link
Contributor

lvyanquan commented Nov 14, 2024

LGTM.
But I still found some changes after runing mvn spotless:apply
image

And a doc update in postgres available-metadata will be more appreciated. You can find the doc in https://github.com/apache/flink-cdc/blob/master/docs/content/docs/connectors/flink-sources/postgres-cdc.md

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