Skip to content

Core: Remove deprecated validation APIs in MergingSnapshotProducer#7150

Merged
jackye1995 merged 1 commit intoapache:masterfrom
amogh-jahagirdar:branch-remove-old-validation-apis
Mar 21, 2023
Merged

Core: Remove deprecated validation APIs in MergingSnapshotProducer#7150
jackye1995 merged 1 commit intoapache:masterfrom
amogh-jahagirdar:branch-remove-old-validation-apis

Conversation

@amogh-jahagirdar
Copy link
Contributor

@amogh-jahagirdar amogh-jahagirdar commented Mar 20, 2023

As part of branch write work, we deprecated the validation APIs in MergingSnapshotProducer in favor of new validation APIs which require explicit passing of the branch. This change removes those deprecated APIs for 1.3.0 and updates rev API.

cc: @rdblue @namrathamyske @jackye1995

@github-actions github-actions bot added the core label Mar 20, 2023
@amogh-jahagirdar amogh-jahagirdar force-pushed the branch-remove-old-validation-apis branch from 03bd312 to df39ad5 Compare March 21, 2023 05:00
@amogh-jahagirdar amogh-jahagirdar changed the title Remove deprecated validation APIs in MergingSnapshotProducer Core: Remove deprecated validation APIs in MergingSnapshotProducer Mar 21, 2023
@nastra
Copy link
Contributor

nastra commented Mar 21, 2023

@amogh-jahagirdar just FYI that you'd need #7155, otherwise the added RevAPI breaks are under the wrong version

@amogh-jahagirdar amogh-jahagirdar force-pushed the branch-remove-old-validation-apis branch from df39ad5 to 85033c1 Compare March 21, 2023 16:16
@amogh-jahagirdar
Copy link
Contributor Author

Good catch @nastra , I just rebased, and now it's under the right version.

@amogh-jahagirdar amogh-jahagirdar force-pushed the branch-remove-old-validation-apis branch from 85033c1 to 0195e7b Compare March 21, 2023 16:23
@amogh-jahagirdar amogh-jahagirdar force-pushed the branch-remove-old-validation-apis branch from 0195e7b to 14300a0 Compare March 21, 2023 16:23
Copy link
Contributor

@nastra nastra left a comment

Choose a reason for hiding this comment

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

this LGTM, thanks @amogh-jahagirdar. I'll wait for this PR to be merged and rebase #7156 then

Copy link
Contributor

@jackye1995 jackye1995 left a comment

Choose a reason for hiding this comment

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

looks good to me

Copy link
Contributor

@singhpk234 singhpk234 left a comment

Choose a reason for hiding this comment

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

LGTM as well, Thanks @amogh-jahagirdar !

@jackye1995
Copy link
Contributor

Thanks for the work @amogh-jahagirdar !

@jackye1995 jackye1995 merged commit 73e2b6c into apache:master Mar 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants