Skip to content

Conversation

@aleksandarskrbic
Copy link
Contributor

@aleksandarskrbic aleksandarskrbic commented Sep 23, 2025

Fixes #648

Why this is a breaking change:

This PR removes Snapshot::metadata, Snapshot::protocol, and Snapshot::column_mapping_mode. These are now all accessed through snapshot.table_configuration

@github-actions github-actions bot added the breaking-change Change that require a major version bump label Sep 23, 2025
@OussamaSaoudi
Copy link
Collaborator

please rename to refactor! since this is a breaking change :)

@codecov
Copy link

codecov bot commented Sep 23, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.91%. Comparing base (e1faff7) to head (1988d84).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1339      +/-   ##
==========================================
+ Coverage   84.90%   84.91%   +0.01%     
==========================================
  Files         113      113              
  Lines       28923    28948      +25     
  Branches    28923    28948      +25     
==========================================
+ Hits        24556    24581      +25     
  Misses       3197     3197              
  Partials     1170     1170              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@aleksandarskrbic aleksandarskrbic changed the title refactor: Snapshot should not expose delta implementation details refactor!: Snapshot should not expose delta implementation details Sep 24, 2025
@OussamaSaoudi
Copy link
Collaborator

Could you rebase please? We'll try to get this in right after :)

@aleksandarskrbic
Copy link
Contributor Author

Could you rebase please? We'll try to get this in right after :)

Done :) @OussamaSaoudi

@OussamaSaoudi
Copy link
Collaborator

Hey, still seeing clippy issues. Take a look at CI and make sure it all passes. ex: cargo clippy --benches --tests --all-features -- -D warnings

@aleksandarskrbic aleksandarskrbic force-pushed the snaphost-should-not-expose-delta-details branch from 6b775fc to 8bbdee7 Compare September 30, 2025 07:30
Copy link
Collaborator

@nicklan nicklan left a comment

Choose a reason for hiding this comment

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

lgtm! thanks!

@OussamaSaoudi
Copy link
Collaborator

ah darn this fell out of sync 😭 could you rebase one more time please?

@aleksandarskrbic aleksandarskrbic force-pushed the snaphost-should-not-expose-delta-details branch from 40d651d to 8bbdee7 Compare October 3, 2025 23:21
@aleksandarskrbic aleksandarskrbic force-pushed the snaphost-should-not-expose-delta-details branch from 8bbdee7 to aa1b0d6 Compare October 3, 2025 23:29
Copy link
Member

@zachschuermann zachschuermann left a comment

Choose a reason for hiding this comment

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

LGTM but can we please make sure to call out breaking changes (or any changes) in PR description

@OussamaSaoudi
Copy link
Collaborator

Updated the description. @aleksandarskrbic in the future please be sure to follow our PR template so that building the changlog is easier.

Thank you for your contribution! :D

@OussamaSaoudi OussamaSaoudi merged commit 01e7cb1 into delta-io:main Oct 7, 2025
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking-change Change that require a major version bump

Projects

None yet

Development

Successfully merging this pull request may close these issues.

refactor!: Snapshot should not expose delta implementation details

4 participants