From 911aa7774b6e0210a0955d7a493d2da82ada54ad Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Mon, 21 Aug 2023 16:31:20 +0200 Subject: [PATCH] Don't return commits from setCommitMergedStatuses Since the slice stores pointers to objects, and we're only modifying the objects but not the slice itself, there's no need to return it and assign it back. This will allow us to call the function for subslices of commits. Also, move the condition that checks for an empty string inside the function; we're going to call it from more than one place, so this makes it easier. --- pkg/commands/git_commands/commit_loader.go | 11 ++++++----- pkg/commands/git_commands/commit_loader_test.go | 3 ++- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/pkg/commands/git_commands/commit_loader.go b/pkg/commands/git_commands/commit_loader.go index 372bc7a4d..b399cff18 100644 --- a/pkg/commands/git_commands/commit_loader.go +++ b/pkg/commands/git_commands/commit_loader.go @@ -137,9 +137,7 @@ func (self *CommitLoader) GetCommits(opts GetCommitsOptions) ([]*models.Commit, return commits, nil } - if ancestor != "" { - commits = setCommitMergedStatuses(ancestor, commits) - } + setCommitMergedStatuses(ancestor, commits) return commits, nil } @@ -495,7 +493,11 @@ func (self *CommitLoader) commitFromPatch(content string) *models.Commit { } } -func setCommitMergedStatuses(ancestor string, commits []*models.Commit) []*models.Commit { +func setCommitMergedStatuses(ancestor string, commits []*models.Commit) { + if ancestor == "" { + return + } + passedAncestor := false for i, commit := range commits { // some commits aren't really commits and don't have sha's, such as the update-ref todo @@ -509,7 +511,6 @@ func setCommitMergedStatuses(ancestor string, commits []*models.Commit) []*model commits[i].Status = models.StatusMerged } } - return commits } func (self *CommitLoader) getMergeBase(refName string) string { diff --git a/pkg/commands/git_commands/commit_loader_test.go b/pkg/commands/git_commands/commit_loader_test.go index 8cf9b95a2..4ae7af94a 100644 --- a/pkg/commands/git_commands/commit_loader_test.go +++ b/pkg/commands/git_commands/commit_loader_test.go @@ -548,7 +548,8 @@ func TestCommitLoader_setCommitMergedStatuses(t *testing.T) { for _, scenario := range scenarios { t.Run(scenario.testName, func(t *testing.T) { - expectedCommits := setCommitMergedStatuses(scenario.ancestor, scenario.commits) + expectedCommits := scenario.commits + setCommitMergedStatuses(scenario.ancestor, expectedCommits) assert.Equal(t, scenario.expectedCommits, expectedCommits) }) }