From 86750244748ed5e3adeaee5296fd68edba4ad1b9 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Tue, 28 Oct 2025 12:08:26 -0700 Subject: [PATCH] Remove unnecessary function parameter --- routers/api/v1/repo/pull.go | 2 +- routers/web/repo/pull.go | 2 +- services/automerge/automerge.go | 2 +- services/pull/merge.go | 4 ++-- services/release/release.go | 2 +- tests/integration/pull_merge_test.go | 29 +++++----------------------- 6 files changed, 11 insertions(+), 30 deletions(-) diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index 9305ad8c2d7b2..68cc5d65a974f 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -1016,7 +1016,7 @@ func MergePullRequest(ctx *context.APIContext) { } } - if err := pull_service.Merge(ctx, pr, ctx.Doer, ctx.Repo.GitRepo, repo_model.MergeStyle(form.Do), form.HeadCommitID, message, false); err != nil { + if err := pull_service.Merge(ctx, pr, ctx.Doer, repo_model.MergeStyle(form.Do), form.HeadCommitID, message, false); err != nil { if pull_service.IsErrInvalidMergeStyle(err) { ctx.APIError(http.StatusMethodNotAllowed, fmt.Errorf("%s is not allowed an allowed merge style for this repository", repo_model.MergeStyle(form.Do))) } else if pull_service.IsErrMergeConflicts(err) { diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index 580339d2cb00b..0f9f551e12e45 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -1152,7 +1152,7 @@ func MergePullRequest(ctx *context.Context) { } } - if err := pull_service.Merge(ctx, pr, ctx.Doer, ctx.Repo.GitRepo, repo_model.MergeStyle(form.Do), form.HeadCommitID, message, false); err != nil { + if err := pull_service.Merge(ctx, pr, ctx.Doer, repo_model.MergeStyle(form.Do), form.HeadCommitID, message, false); err != nil { if pull_service.IsErrInvalidMergeStyle(err) { ctx.JSONError(ctx.Tr("repo.pulls.invalid_merge_option")) } else if pull_service.IsErrMergeConflicts(err) { diff --git a/services/automerge/automerge.go b/services/automerge/automerge.go index f14911f880db8..e145f93f044aa 100644 --- a/services/automerge/automerge.go +++ b/services/automerge/automerge.go @@ -260,7 +260,7 @@ func handlePullRequestAutoMerge(pullID int64, sha string) { return } - if err := pull_service.Merge(ctx, pr, doer, baseGitRepo, scheduledPRM.MergeStyle, "", scheduledPRM.Message, true); err != nil { + if err := pull_service.Merge(ctx, pr, doer, scheduledPRM.MergeStyle, "", scheduledPRM.Message, true); err != nil { log.Error("pull_service.Merge: %v", err) // FIXME: if merge failed, we should display some error message to the pull request page. // The resolution is add a new column on automerge table named `error_message` to store the error message and displayed diff --git a/services/pull/merge.go b/services/pull/merge.go index 43bb3dd235c52..9c7e09a227e1d 100644 --- a/services/pull/merge.go +++ b/services/pull/merge.go @@ -220,7 +220,7 @@ func (err ErrInvalidMergeStyle) Unwrap() error { // Merge merges pull request to base repository. // Caller should check PR is ready to be merged (review and status checks) -func Merge(ctx context.Context, pr *issues_model.PullRequest, doer *user_model.User, baseGitRepo *git.Repository, mergeStyle repo_model.MergeStyle, expectedHeadCommitID, message string, wasAutoMerged bool) error { +func Merge(ctx context.Context, pr *issues_model.PullRequest, doer *user_model.User, mergeStyle repo_model.MergeStyle, expectedHeadCommitID, message string, wasAutoMerged bool) error { if err := pr.LoadBaseRepo(ctx); err != nil { log.Error("Unable to load base repo: %v", err) return fmt.Errorf("unable to load base repo: %w", err) @@ -668,7 +668,7 @@ func MergedManually(ctx context.Context, pr *issues_model.PullRequest, doer *use return err } - notify_service.MergePullRequest(baseGitRepo.Ctx, doer, pr) + notify_service.MergePullRequest(ctx, doer, pr) log.Info("manuallyMerged[%d]: Marked as manually merged into %s/%s by commit id: %s", pr.ID, pr.BaseRepo.Name, pr.BaseBranch, commitID) return handleCloseCrossReferences(ctx, pr, doer) diff --git a/services/release/release.go b/services/release/release.go index 28061ae8b180c..29df66e5e6640 100644 --- a/services/release/release.go +++ b/services/release/release.go @@ -262,7 +262,7 @@ func UpdateRelease(ctx context.Context, doer *user_model.User, gitRepo *git.Repo if rel.ID == 0 { return errors.New("UpdateRelease only accepts an exist release") } - isTagCreated, err := createTag(gitRepo.Ctx, gitRepo, rel, "") + isTagCreated, err := createTag(ctx, gitRepo, rel, "") if err != nil { return err } diff --git a/tests/integration/pull_merge_test.go b/tests/integration/pull_merge_test.go index 32ab7413829c7..062be3ae7a198 100644 --- a/tests/integration/pull_merge_test.go +++ b/tests/integration/pull_merge_test.go @@ -325,17 +325,13 @@ func TestCantMergeConflict(t *testing.T) { BaseBranch: "base", }) - gitRepo, err := gitrepo.OpenRepository(t.Context(), repo1) - assert.NoError(t, err) - - err = pull_service.Merge(t.Context(), pr, user1, gitRepo, repo_model.MergeStyleMerge, "", "CONFLICT", false) + err := pull_service.Merge(t.Context(), pr, user1, repo_model.MergeStyleMerge, "", "CONFLICT", false) assert.Error(t, err, "Merge should return an error due to conflict") assert.True(t, pull_service.IsErrMergeConflicts(err), "Merge error is not a conflict error") - err = pull_service.Merge(t.Context(), pr, user1, gitRepo, repo_model.MergeStyleRebase, "", "CONFLICT", false) + err = pull_service.Merge(t.Context(), pr, user1, repo_model.MergeStyleRebase, "", "CONFLICT", false) assert.Error(t, err, "Merge should return an error due to conflict") assert.True(t, pull_service.IsErrRebaseConflicts(err), "Merge error is not a conflict error") - gitRepo.Close() }) } @@ -423,8 +419,6 @@ func TestCantMergeUnrelated(t *testing.T) { session.MakeRequest(t, req, http.StatusCreated) // Now this PR could be marked conflict - or at least a race may occur - so drop down to pure code at this point... - gitRepo, err := gitrepo.OpenRepository(t.Context(), repo1) - assert.NoError(t, err) pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ HeadRepoID: repo1.ID, BaseRepoID: repo1.ID, @@ -432,10 +426,9 @@ func TestCantMergeUnrelated(t *testing.T) { BaseBranch: "base", }) - err = pull_service.Merge(t.Context(), pr, user1, gitRepo, repo_model.MergeStyleMerge, "", "UNRELATED", false) + err = pull_service.Merge(t.Context(), pr, user1, repo_model.MergeStyleMerge, "", "UNRELATED", false) assert.Error(t, err, "Merge should return an error due to unrelated") assert.True(t, pull_service.IsErrMergeUnrelatedHistories(err), "Merge error is not a unrelated histories error") - gitRepo.Close() }) } @@ -469,14 +462,8 @@ func TestFastForwardOnlyMerge(t *testing.T) { BaseBranch: "master", }) - gitRepo, err := git.OpenRepository(t.Context(), repo_model.RepoPath(user1.Name, repo1.Name)) - assert.NoError(t, err) - - err = pull_service.Merge(t.Context(), pr, user1, gitRepo, repo_model.MergeStyleFastForwardOnly, "", "FAST-FORWARD-ONLY", false) - + err := pull_service.Merge(t.Context(), pr, user1, repo_model.MergeStyleFastForwardOnly, "", "FAST-FORWARD-ONLY", false) assert.NoError(t, err) - - gitRepo.Close() }) } @@ -511,15 +498,9 @@ func TestCantFastForwardOnlyMergeDiverging(t *testing.T) { BaseBranch: "master", }) - gitRepo, err := git.OpenRepository(t.Context(), repo_model.RepoPath(user1.Name, repo1.Name)) - assert.NoError(t, err) - - err = pull_service.Merge(t.Context(), pr, user1, gitRepo, repo_model.MergeStyleFastForwardOnly, "", "DIVERGING", false) - + err := pull_service.Merge(t.Context(), pr, user1, repo_model.MergeStyleFastForwardOnly, "", "DIVERGING", false) assert.Error(t, err, "Merge should return an error due to being for a diverging branch") assert.True(t, pull_service.IsErrMergeDivergingFastForwardOnly(err), "Merge error is not a diverging fast-forward-only error") - - gitRepo.Close() }) }