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.

Sign up for GitHub

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

Jump to bottom

Rename CdfFileEntry to CdcEntry #17505

Merged
merged 1 commit into from
May 16, 2023
Merged

Rename CdfFileEntry to CdcEntry #17505

merged 1 commit into from
May 16, 2023

Conversation

marcinsbd
Copy link
Contributor

@marcinsbd marcinsbd commented May 15, 2023 •
edited
Loading

Description

Fixes #17183

@cla-bot cla-bot bot added the cla-signed label May 15, 2023
@marcinsbd marcinsbd requested a review from homar May 15, 2023 12:26
@homar homar requested a review from findepi May 15, 2023 12:32
@marcinsbd marcinsbd force-pushed the rename_CdfFileEntry_to_CdcEntry branch from 037e316 to 68d1b48 Compare May 15, 2023 13:05
@github-actions github-actions bot added the delta-lake Delta Lake connector label May 15, 2023
ebyhr
ebyhr reviewed May 15, 2023
View reviewed changes
Copy link
Member

@ebyhr ebyhr left a comment

Choose a reason for hiding this comment

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

There're still many "cdf" usages in the connector. Did you leave it intentionally? What's the criteria to use cdf or cdc in the code?

@ebyhr
Copy link
Member

ebyhr commented May 15, 2023 •
edited
Loading

Fix issue: #17183

This comment doesn't close the issue automatically. Please use the template in https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword. e.g. Fixes #17183

Release notes
( ) This is not user-visible or docs only and no release notes are required.
( ) Release notes are required, please propose a release note for me.
( ) Release notes are required, with the following suggested text:
...

Please remove unrelated part so that reviewers don't need to read it.

@homar
Copy link
Member

homar commented May 16, 2023

There're still many "cdf" usages in the connector. Did you leave it intentionally? What's the criteria to use cdf or cdc in the code?

So my understanding is that the name of the feature is CDF (change data feed) and databricks also uses this name for the feature, however the name of the entry in the log is for some reason cdc. The idea was to only rename parts that directly touches that cdc log entry entity.
Does it make sese? WDYT ?

@marcinsbd marcinsbd force-pushed the rename_CdfFileEntry_to_CdcEntry branch from 68d1b48 to d2c483b Compare May 16, 2023 14:45
@marcinsbd marcinsbd requested review from homar and ebyhr May 16, 2023 17:09
@ebyhr ebyhr force-pushed the rename_CdfFileEntry_to_CdcEntry branch from d2c483b to 56056df Compare May 16, 2023 21:44
@ebyhr ebyhr merged commit 50ea0ba into trinodb:master May 16, 2023
@github-actions github-actions bot added this to the 418 milestone May 16, 2023
@colebow colebow mentioned this pull request May 17, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@findepi findepi Awaiting requested review from findepi

@homar homar Awaiting requested review from homar

@ebyhr ebyhr Awaiting requested review from ebyhr

Assignees
No one assigned
Labels
cla-signed delta-lake Delta Lake connector
Milestone
418
Development

Successfully merging this pull request may close these issues.

Rename CdfFileEntry to CdcEntry
3 participants
@marcinsbd @ebyhr @homar