Skip to content

Commit aa4effc

Browse files
johnnyaugjohnnyaug
authored
fix arg order in merge cmd (treeverse#1341)
1 parent 0e7d562 commit aa4effc

File tree

14 files changed

+67
-68
lines changed

14 files changed

+67
-68
lines changed

‎api/client.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ type RepositoryClient interface {
8282
DeleteObject(ctx context.Context, repository, branchID, path string) error
8383

8484
DiffRefs(ctx context.Context, repository, leftRef, rightRef string, after string, amount int) ([]*models.Diff, *models.Pagination, error)
85-
Merge(ctx context.Context, repository, leftRef, rightRef string) (*models.MergeResult, error)
85+
Merge(ctx context.Context, repository, destinationBranch, sourceRef string) (*models.MergeResult, error)
8686

8787
DiffBranch(ctx context.Context, repository, branch string, after string, amount int) ([]*models.Diff, *models.Pagination, error)
8888

@@ -554,12 +554,12 @@ func (c *client) DiffRefs(ctx context.Context, repository, leftRef, rightRef, af
554554
return payload.Results, payload.Pagination, nil
555555
}
556556

557-
func (c *client) Merge(ctx context.Context, repository, leftRef, rightRef string) (*models.MergeResult, error) {
557+
func (c *client) Merge(ctx context.Context, repository, destinationBranch, sourceRef string) (*models.MergeResult, error) {
558558
statusOK, err := c.remote.Refs.MergeIntoBranch(&refs.MergeIntoBranchParams{
559-
DestinationRef: leftRef,
560-
SourceRef: rightRef,
561-
Repository: repository,
562-
Context: ctx,
559+
DestinationBranch: destinationBranch,
560+
SourceRef: sourceRef,
561+
Repository: repository,
562+
Context: ctx,
563563
}, c.auth)
564564

565565
if err == nil {

‎api/controller.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -825,7 +825,7 @@ func (c *Controller) MergeMergeIntoBranchHandler() refs.MergeIntoBranchHandler {
825825
deps, err := c.setupRequest(user, params.HTTPRequest, []permissions.Permission{
826826
{
827827
Action: permissions.CreateCommitAction,
828-
Resource: permissions.BranchArn(params.Repository, params.DestinationRef),
828+
Resource: permissions.BranchArn(params.Repository, params.DestinationBranch),
829829
},
830830
})
831831
if err != nil {
@@ -843,7 +843,7 @@ func (c *Controller) MergeMergeIntoBranchHandler() refs.MergeIntoBranchHandler {
843843
metadata = params.Merge.Metadata
844844
}
845845
res, err := deps.Cataloger.Merge(deps.ctx,
846-
params.Repository, params.SourceRef, params.DestinationRef,
846+
params.Repository, params.DestinationBranch, params.SourceRef,
847847
userModel.Username,
848848
message,
849849
metadata)

‎benchmarks/main.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,7 @@ func uploader(ctx context.Context, ch chan string, repoName, contentPrefix strin
189189
func merge(ctx context.Context) {
190190
err := retry.Do(func() error {
191191
_, err := client.Refs.MergeIntoBranch(&refs.MergeIntoBranchParams{
192-
DestinationRef: "master",
192+
DestinationBranch: "master",
193193
Merge: &models.Merge{
194194
Message: "merging all objects to master",
195195
},

‎catalog/cataloger.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ type Cataloger interface {
9696
Compare(ctx context.Context, repository, leftReference string, rightReference string, params DiffParams) (Differences, bool, error)
9797
DiffUncommitted(ctx context.Context, repository, branch string, limit int, after string) (Differences, bool, error)
9898

99-
Merge(ctx context.Context, repository, leftBranch, rightBranch, committer, message string, metadata Metadata) (*MergeResult, error)
99+
Merge(ctx context.Context, repository, destinationBranch, sourceRef, committer, message string, metadata Metadata) (*MergeResult, error)
100100

101101
Hooks() *CatalogerHooks
102102

‎catalog/entry_catalog.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -354,20 +354,20 @@ func (e *EntryCatalog) Revert(ctx context.Context, repositoryID graveler.Reposit
354354
return e.Store.Revert(ctx, repositoryID, branchID, ref, parentNumber, commitParams)
355355
}
356356

357-
func (e *EntryCatalog) Merge(ctx context.Context, repositoryID graveler.RepositoryID, from graveler.Ref, to graveler.BranchID, commitParams graveler.CommitParams) (graveler.CommitID, graveler.DiffSummary, error) {
357+
func (e *EntryCatalog) Merge(ctx context.Context, repositoryID graveler.RepositoryID, destination graveler.BranchID, source graveler.Ref, commitParams graveler.CommitParams) (graveler.CommitID, graveler.DiffSummary, error) {
358358
if commitParams.Message == "" {
359-
commitParams.Message = fmt.Sprintf("Merge '%s' into '%s'", from, to)
359+
commitParams.Message = fmt.Sprintf("Merge '%s' into '%s'", source, destination)
360360
}
361361
if err := Validate([]ValidateArg{
362362
{"repositoryID", repositoryID, ValidateRepositoryID},
363-
{"from", from, ValidateRef},
364-
{"to", to, ValidateBranchID},
363+
{"destination", destination, ValidateBranchID},
364+
{"source", source, ValidateRef},
365365
{"committer", commitParams.Committer, ValidateRequiredString},
366366
{"message", commitParams.Message, ValidateRequiredString},
367367
}); err != nil {
368368
return "", graveler.DiffSummary{}, err
369369
}
370-
return e.Store.Merge(ctx, repositoryID, from, to, commitParams)
370+
return e.Store.Merge(ctx, repositoryID, destination, source, commitParams)
371371
}
372372

373373
func (e *EntryCatalog) DiffUncommitted(ctx context.Context, repositoryID graveler.RepositoryID, branchID graveler.BranchID) (EntryDiffIterator, error) {

‎catalog/fake_graveler_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,7 @@ func (g *FakeGraveler) Revert(_ context.Context, _ graveler.RepositoryID, _ grav
163163
panic("implement me")
164164
}
165165

166-
func (g *FakeGraveler) Merge(ctx context.Context, repositoryID graveler.RepositoryID, from graveler.Ref, to graveler.BranchID, _ graveler.CommitParams) (graveler.CommitID, graveler.DiffSummary, error) {
166+
func (g *FakeGraveler) Merge(ctx context.Context, repositoryID graveler.RepositoryID, destination graveler.BranchID, source graveler.Ref, _ graveler.CommitParams) (graveler.CommitID, graveler.DiffSummary, error) {
167167
panic("implement me")
168168
}
169169

‎catalog/rocks_cataloger.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -598,12 +598,12 @@ func listDiffHelper(it EntryDiffIterator, limit int, after string) (Differences,
598598
return diffs, hasMore, nil
599599
}
600600

601-
func (c *cataloger) Merge(ctx context.Context, repository string, leftBranch string, rightBranch string, committer string, message string, metadata Metadata) (*MergeResult, error) {
601+
func (c *cataloger) Merge(ctx context.Context, repository string, destinationBranch string, sourceRef string, committer string, message string, metadata Metadata) (*MergeResult, error) {
602602
repositoryID := graveler.RepositoryID(repository)
603-
leftRef := graveler.Ref(leftBranch)
604-
rightBranchID := graveler.BranchID(rightBranch)
603+
dest := graveler.BranchID(destinationBranch)
604+
source := graveler.Ref(sourceRef)
605605
meta := graveler.Metadata(metadata)
606-
commitID, summary, err := c.EntryCatalog.Merge(ctx, repositoryID, leftRef, rightBranchID, graveler.CommitParams{
606+
commitID, summary, err := c.EntryCatalog.Merge(ctx, repositoryID, dest, source, graveler.CommitParams{
607607
Committer: committer,
608608
Message: message,
609609
Metadata: meta,

‎cmd/lakectl/cmd/merge.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -41,14 +41,14 @@ var mergeCmd = &cobra.Command{
4141
),
4242
Run: func(cmd *cobra.Command, args []string) {
4343
client := getClient()
44-
rightRefURI := uri.Must(uri.Parse(args[0]))
45-
leftRefURI := uri.Must(uri.Parse(args[1]))
44+
sourceRef := uri.Must(uri.Parse(args[0]))
45+
destinationRef := uri.Must(uri.Parse(args[1]))
4646

47-
if leftRefURI.Repository != rightRefURI.Repository {
47+
if destinationRef.Repository != sourceRef.Repository {
4848
Die("both references must belong to the same repository", 1)
4949
}
5050

51-
result, err := client.Merge(context.Background(), leftRefURI.Repository, leftRefURI.Ref, rightRefURI.Ref)
51+
result, err := client.Merge(context.Background(), destinationRef.Repository, destinationRef.Ref, sourceRef.Ref)
5252
if errors.Is(err, catalog.ErrConflictFound) {
5353
_, _ = fmt.Printf("Conflicts: %d\n", result.Summary.Conflict)
5454
return
@@ -61,7 +61,7 @@ var mergeCmd = &cobra.Command{
6161
Merge FromTo
6262
Result *models.MergeResult
6363
}{
64-
FromTo{FromRef: leftRefURI.Ref, ToRef: rightRefURI.Ref},
64+
FromTo{FromRef: sourceRef.Ref, ToRef: destinationRef.Ref},
6565
result,
6666
})
6767
},

‎graveler/committed/manager.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -92,8 +92,8 @@ func (c *committedManager) Diff(ctx context.Context, ns graveler.StorageNamespac
9292
return NewDiffIterator(leftIt, rightIt), nil
9393
}
9494

95-
func (c *committedManager) Merge(ctx context.Context, ns graveler.StorageNamespace, theirs, ours, base graveler.MetaRangeID) (graveler.MetaRangeID, graveler.DiffSummary, error) {
96-
diffIt, err := c.Diff(ctx, ns, theirs, ours)
95+
func (c *committedManager) Merge(ctx context.Context, ns graveler.StorageNamespace, destination, source, base graveler.MetaRangeID) (graveler.MetaRangeID, graveler.DiffSummary, error) {
96+
diffIt, err := c.Diff(ctx, ns, destination, source)
9797
if err != nil {
9898
return "", graveler.DiffSummary{}, fmt.Errorf("diff: %w", err)
9999
}
@@ -105,7 +105,7 @@ func (c *committedManager) Merge(ctx context.Context, ns graveler.StorageNamespa
105105
defer baseIt.Close()
106106
patchIterator := NewMergeIterator(diffIt, baseIt)
107107
defer patchIterator.Close()
108-
return c.Apply(ctx, ns, theirs, patchIterator)
108+
return c.Apply(ctx, ns, destination, patchIterator)
109109
}
110110

111111
func (c *committedManager) Apply(ctx context.Context, ns graveler.StorageNamespace, rangeID graveler.MetaRangeID, diffs graveler.ValueIterator) (graveler.MetaRangeID, graveler.DiffSummary, error) {
@@ -135,8 +135,8 @@ func (c *committedManager) Apply(ctx context.Context, ns graveler.StorageNamespa
135135
return *newID, summary, err
136136
}
137137

138-
func (c *committedManager) Compare(ctx context.Context, ns graveler.StorageNamespace, theirs, ours, base graveler.MetaRangeID) (graveler.DiffIterator, error) {
139-
diffIt, err := c.Diff(ctx, ns, theirs, ours)
138+
func (c *committedManager) Compare(ctx context.Context, ns graveler.StorageNamespace, destination, source, base graveler.MetaRangeID) (graveler.DiffIterator, error) {
139+
diffIt, err := c.Diff(ctx, ns, destination, source)
140140
if err != nil {
141141
return nil, fmt.Errorf("diff: %w", err)
142142
}

‎graveler/committed/merge_iterator.go

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -18,20 +18,20 @@ type compareValueIterator struct {
1818
*compareIterator
1919
}
2020

21-
// NewMergeIterator accepts an iterator describing a diff from theirs to ours.
22-
// It returns a graveler.ValueIterator with the changes to perform on theirs, in order to merge ours into it,
21+
// NewMergeIterator accepts an iterator describing a diff from the merge destination to the source.
22+
// It returns a graveler.ValueIterator with the changes to perform on the destination branch, in order to merge the source into it,
2323
// relative to base as the merge base.
2424
// When reaching a conflict, the iterator will enter an error state with the graveler.ErrConflictFound error.
25-
func NewMergeIterator(diffTheirsToOurs graveler.DiffIterator, base Iterator) *compareValueIterator {
26-
return &compareValueIterator{compareIterator: &compareIterator{diffIt: diffTheirsToOurs, base: base, errorOnConflict: true}}
25+
func NewMergeIterator(diffDestToSource graveler.DiffIterator, base Iterator) *compareValueIterator {
26+
return &compareValueIterator{compareIterator: &compareIterator{diffIt: diffDestToSource, base: base, errorOnConflict: true}}
2727
}
2828

29-
// NewCompareIterator accepts an iterator describing a diff from theirs to ours.
30-
// It returns a graveler.DiffIterator with the changes to perform on theirs, in order to merge ours into it,
29+
// NewCompareIterator accepts an iterator describing a diff from the merge destination to the source.
30+
// It returns a graveler.DiffIterator with the changes to perform on the destination branch, in order to merge the source into it,
3131
// relative to base as the merge base.
3232
// When reaching a conflict, the returned Diff will be of type graveler.DiffTypeConflict.
33-
func NewCompareIterator(diffTheirsToOurs graveler.DiffIterator, base Iterator) *compareIterator {
34-
return &compareIterator{diffIt: diffTheirsToOurs, base: base, errorOnConflict: false}
33+
func NewCompareIterator(diffDestToSource graveler.DiffIterator, base Iterator) *compareIterator {
34+
return &compareIterator{diffIt: diffDestToSource, base: base, errorOnConflict: false}
3535
}
3636

3737
func (d *compareIterator) valueFromBase(key graveler.Key) (*graveler.ValueRecord, error) {
@@ -71,45 +71,45 @@ func (d *compareIterator) Next() bool {
7171
}
7272
switch typ {
7373
case graveler.DiffTypeAdded:
74-
// exists on ours, but not on theirs
74+
// exists on source, but not on dest
7575
if baseVal == nil {
76-
// added only on ours
76+
// added only on source
7777
d.val = d.diffIt.Value().Copy()
7878
return true
7979
}
8080
if !bytes.Equal(baseVal.Identity, val.Value.Identity) {
81-
// removed on theirs, but changed on ours
81+
// removed on dest, but changed on source
8282
return d.handleConflict()
8383
}
8484
continue
8585
case graveler.DiffTypeChanged:
8686
if baseVal == nil {
87-
// added on theirs and ours, with different identities
87+
// added on dest and source, with different identities
8888
return d.handleConflict()
8989
}
9090
if bytes.Equal(baseVal.Identity, val.Value.Identity) {
91-
// changed on theirs, but not on ours
91+
// changed on dest, but not on source
9292
continue
9393
}
9494
if !bytes.Equal(baseVal.Identity, val.LeftIdentity) {
95-
// changed on theirs and ours, to different identities
95+
// changed on dest and source, to different identities
9696
return d.handleConflict()
9797
}
98-
// changed only on ours
98+
// changed only on source
9999
d.val = d.diffIt.Value().Copy()
100100
return true
101101
case graveler.DiffTypeRemoved:
102-
// exists on theirs, but not on ours
102+
// exists on dest, but not on source
103103
if baseVal != nil {
104104
if bytes.Equal(baseVal.Identity, val.LeftIdentity) {
105-
// removed on ours, not changed on theirs
105+
// removed on source, not changed on dest
106106
d.val = d.diffIt.Value().Copy()
107107
return true
108108
}
109-
// changed on theirs, removed on ours
109+
// changed on dest, removed on source
110110
return d.handleConflict()
111111
}
112-
// added on theirs, but not on ours - continue
112+
// added on dest, but not on source - continue
113113
}
114114
}
115115
d.err = d.diffIt.Err()

0 commit comments

Comments
 (0)