Dont overwrite err with nil & rename PullCheckingFuncs to reflect there usage (#19572)
- dont overwrite err with nil unintentionaly - rename CheckPRReadyToMerge to CheckPullBranchProtections - rename prQueue to prPatchCheckerQueue from #9307 Co-authored-by: delvh <dev.lh@web.de>
This commit is contained in:
parent
3725fa28cc
commit
d8905cb623
|
@ -343,7 +343,7 @@ func preReceiveBranch(ctx *preReceiveContext, oldCommitID, newCommitID, refFullN
|
|||
}
|
||||
|
||||
// Check all status checks and reviews are ok
|
||||
if err := pull_service.CheckPRReadyToMerge(ctx, pr, true); err != nil {
|
||||
if err := pull_service.CheckPullBranchProtections(ctx, pr, true); err != nil {
|
||||
if models.IsErrDisallowedToMerge(err) {
|
||||
log.Warn("Forbidden: User %d is not allowed push to protected branch %s in %-v and pr #%d is not ready to be merged: %s", ctx.opts.UserID, branchName, repo, pr.Index, err.Error())
|
||||
ctx.JSON(http.StatusForbidden, private.Response{
|
||||
|
|
|
@ -28,12 +28,12 @@ import (
|
|||
asymkey_service "code.gitea.io/gitea/services/asymkey"
|
||||
)
|
||||
|
||||
// prQueue represents a queue to handle update pull request tests
|
||||
var prQueue queue.UniqueQueue
|
||||
// prPatchCheckerQueue represents a queue to handle update pull request tests
|
||||
var prPatchCheckerQueue queue.UniqueQueue
|
||||
|
||||
var (
|
||||
ErrIsClosed = errors.New("pull is cosed")
|
||||
ErrUserNotAllowedToMerge = errors.New("user not allowed to merge")
|
||||
ErrIsClosed = errors.New("pull is closed")
|
||||
ErrUserNotAllowedToMerge = models.ErrDisallowedToMerge{}
|
||||
ErrHasMerged = errors.New("has already been merged")
|
||||
ErrIsWorkInProgress = errors.New("work in progress PRs cannot be merged")
|
||||
ErrIsChecking = errors.New("cannot merge while conflict checking is in progress")
|
||||
|
@ -43,7 +43,7 @@ var (
|
|||
|
||||
// AddToTaskQueue adds itself to pull request test task queue.
|
||||
func AddToTaskQueue(pr *models.PullRequest) {
|
||||
err := prQueue.PushFunc(strconv.FormatInt(pr.ID, 10), func() error {
|
||||
err := prPatchCheckerQueue.PushFunc(strconv.FormatInt(pr.ID, 10), func() error {
|
||||
pr.Status = models.PullRequestStatusChecking
|
||||
err := pr.UpdateColsIfNotMerged("status")
|
||||
if err != nil {
|
||||
|
@ -93,13 +93,13 @@ func CheckPullMergable(ctx context.Context, doer *user_model.User, perm *models.
|
|||
return ErrIsChecking
|
||||
}
|
||||
|
||||
if err := CheckPRReadyToMerge(ctx, pr, false); err != nil {
|
||||
if err := CheckPullBranchProtections(ctx, pr, false); err != nil {
|
||||
if models.IsErrDisallowedToMerge(err) {
|
||||
if force {
|
||||
if isRepoAdmin, err := models.IsUserRepoAdmin(pr.BaseRepo, doer); err != nil {
|
||||
return err
|
||||
if isRepoAdmin, err2 := models.IsUserRepoAdmin(pr.BaseRepo, doer); err2 != nil {
|
||||
return err2
|
||||
} else if !isRepoAdmin {
|
||||
return ErrUserNotAllowedToMerge
|
||||
return err
|
||||
}
|
||||
}
|
||||
} else {
|
||||
|
@ -144,7 +144,7 @@ func checkAndUpdateStatus(pr *models.PullRequest) {
|
|||
}
|
||||
|
||||
// Make sure there is no waiting test to process before leaving the checking status.
|
||||
has, err := prQueue.Has(strconv.FormatInt(pr.ID, 10))
|
||||
has, err := prPatchCheckerQueue.Has(strconv.FormatInt(pr.ID, 10))
|
||||
if err != nil {
|
||||
log.Error("Unable to check if the queue is waiting to reprocess pr.ID %d. Error: %v", pr.ID, err)
|
||||
}
|
||||
|
@ -293,7 +293,7 @@ func InitializePullRequests(ctx context.Context) {
|
|||
case <-ctx.Done():
|
||||
return
|
||||
default:
|
||||
if err := prQueue.PushFunc(strconv.FormatInt(prID, 10), func() error {
|
||||
if err := prPatchCheckerQueue.PushFunc(strconv.FormatInt(prID, 10), func() error {
|
||||
log.Trace("Adding PR ID: %d to the pull requests patch checking queue", prID)
|
||||
return nil
|
||||
}); err != nil {
|
||||
|
@ -358,13 +358,13 @@ func CheckPrsForBaseBranch(baseRepo *repo_model.Repository, baseBranchName strin
|
|||
|
||||
// Init runs the task queue to test all the checking status pull requests
|
||||
func Init() error {
|
||||
prQueue = queue.CreateUniqueQueue("pr_patch_checker", handle, "")
|
||||
prPatchCheckerQueue = queue.CreateUniqueQueue("pr_patch_checker", handle, "")
|
||||
|
||||
if prQueue == nil {
|
||||
if prPatchCheckerQueue == nil {
|
||||
return fmt.Errorf("Unable to create pr_patch_checker Queue")
|
||||
}
|
||||
|
||||
go graceful.GetManager().RunWithShutdownFns(prQueue.Run)
|
||||
go graceful.GetManager().RunWithShutdownFns(prPatchCheckerQueue.Run)
|
||||
go graceful.GetManager().RunWithShutdownContext(InitializePullRequests)
|
||||
return nil
|
||||
}
|
||||
|
|
|
@ -41,7 +41,7 @@ func TestPullRequest_AddToTaskQueue(t *testing.T) {
|
|||
queueShutdown := []func(){}
|
||||
queueTerminate := []func(){}
|
||||
|
||||
prQueue = q.(queue.UniqueQueue)
|
||||
prPatchCheckerQueue = q.(queue.UniqueQueue)
|
||||
|
||||
pr := unittest.AssertExistsAndLoadBean(t, &models.PullRequest{ID: 2}).(*models.PullRequest)
|
||||
AddToTaskQueue(pr)
|
||||
|
@ -51,11 +51,11 @@ func TestPullRequest_AddToTaskQueue(t *testing.T) {
|
|||
return pr.Status == models.PullRequestStatusChecking
|
||||
}, 1*time.Second, 100*time.Millisecond)
|
||||
|
||||
has, err := prQueue.Has(strconv.FormatInt(pr.ID, 10))
|
||||
has, err := prPatchCheckerQueue.Has(strconv.FormatInt(pr.ID, 10))
|
||||
assert.True(t, has)
|
||||
assert.NoError(t, err)
|
||||
|
||||
prQueue.Run(func(shutdown func()) {
|
||||
prPatchCheckerQueue.Run(func(shutdown func()) {
|
||||
queueShutdown = append(queueShutdown, shutdown)
|
||||
}, func(terminate func()) {
|
||||
queueTerminate = append(queueTerminate, terminate)
|
||||
|
@ -68,7 +68,7 @@ func TestPullRequest_AddToTaskQueue(t *testing.T) {
|
|||
assert.Fail(t, "Timeout: nothing was added to pullRequestQueue")
|
||||
}
|
||||
|
||||
has, err = prQueue.Has(strconv.FormatInt(pr.ID, 10))
|
||||
has, err = prPatchCheckerQueue.Has(strconv.FormatInt(pr.ID, 10))
|
||||
assert.False(t, has)
|
||||
assert.NoError(t, err)
|
||||
|
||||
|
@ -82,5 +82,5 @@ func TestPullRequest_AddToTaskQueue(t *testing.T) {
|
|||
callback()
|
||||
}
|
||||
|
||||
prQueue = nil
|
||||
prPatchCheckerQueue = nil
|
||||
}
|
||||
|
|
|
@ -662,8 +662,8 @@ func IsUserAllowedToMerge(pr *models.PullRequest, p models.Permission, user *use
|
|||
return false, nil
|
||||
}
|
||||
|
||||
// CheckPRReadyToMerge checks whether the PR is ready to be merged (reviews and status checks)
|
||||
func CheckPRReadyToMerge(ctx context.Context, pr *models.PullRequest, skipProtectedFilesCheck bool) (err error) {
|
||||
// CheckPullBranchProtections checks whether the PR is ready to be merged (reviews and status checks)
|
||||
func CheckPullBranchProtections(ctx context.Context, pr *models.PullRequest, skipProtectedFilesCheck bool) (err error) {
|
||||
if err = pr.LoadBaseRepoCtx(ctx); err != nil {
|
||||
return fmt.Errorf("LoadBaseRepo: %v", err)
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue