Skip to content

Conversation

itaigilo
Copy link
Contributor

@itaigilo itaigilo commented Oct 6, 2025

Remove a param which is unused by any of the consumers (OSS or Enterprise).

@itaigilo itaigilo requested review from a team and guy-har October 6, 2025 08:38
@itaigilo itaigilo added exclude-changelog PR description should not be included in next release changelog minor-change Used for PRs that don't require issue attached labels Oct 6, 2025
}

func (cr *ConflictResolverWrapper) ResolveConflict(ctx context.Context, sCtx graveler.StorageContext, strategy graveler.MergeStrategy, srcValue, destValue *graveler.ValueRecord) (*graveler.ValueRecord, error) {
func (cr *ConflictResolverWrapper) ResolveConflict(ctx context.Context, sCtx graveler.StorageContext, _ graveler.MergeStrategy, srcValue, destValue *graveler.ValueRecord) (*graveler.ValueRecord, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

qq, if we're removing the strategy from the interface, why do you keep it in the ConflictResolverWrapper implementation? wouldn't that break its interface compatibility.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The strategy is removed from the catalog.EntryConflictResolver interface,
While ConflictResolverWrapper implements the graveler.ConflictResolver interface.

The only struct that implements catalog.EntryConflictResolver is in the Enterprise.

@itaigilo itaigilo requested review from a team and AliRamberg October 8, 2025 12:36
@itaigilo itaigilo merged commit dbc3ba8 into master Oct 9, 2025
44 checks passed
@itaigilo itaigilo deleted the fix/remove-strategy-from-entry-conflict-resolver branch October 9, 2025 15:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

exclude-changelog PR description should not be included in next release changelog minor-change Used for PRs that don't require issue attached

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants