-
-
Notifications
You must be signed in to change notification settings - Fork 5.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add option to API to update PullRequest base branch #11666
Add option to API to update PullRequest base branch #11666
Conversation
Failing test seem unrelated? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some things I took from:
Lines 1190 to 1229 in 5cb201d
if err := pull_service.ChangeTargetBranch(pr, ctx.User, targetBranch); err != nil { | |
if models.IsErrPullRequestAlreadyExists(err) { | |
err := err.(models.ErrPullRequestAlreadyExists) | |
RepoRelPath := ctx.Repo.Owner.Name + "/" + ctx.Repo.Repository.Name | |
errorMessage := ctx.Tr("repo.pulls.has_pull_request", ctx.Repo.RepoLink, RepoRelPath, err.IssueID) | |
ctx.Flash.Error(errorMessage) | |
ctx.JSON(http.StatusConflict, map[string]interface{}{ | |
"error": err.Error(), | |
"user_error": errorMessage, | |
}) | |
} else if models.IsErrIssueIsClosed(err) { | |
errorMessage := ctx.Tr("repo.pulls.is_closed") | |
ctx.Flash.Error(errorMessage) | |
ctx.JSON(http.StatusConflict, map[string]interface{}{ | |
"error": err.Error(), | |
"user_error": errorMessage, | |
}) | |
} else if models.IsErrPullRequestHasMerged(err) { | |
errorMessage := ctx.Tr("repo.pulls.has_merged") | |
ctx.Flash.Error(errorMessage) | |
ctx.JSON(http.StatusConflict, map[string]interface{}{ | |
"error": err.Error(), | |
"user_error": errorMessage, | |
}) | |
} else if models.IsErrBranchesEqual(err) { | |
errorMessage := ctx.Tr("repo.pulls.nothing_to_compare") | |
ctx.Flash.Error(errorMessage) | |
ctx.JSON(http.StatusBadRequest, map[string]interface{}{ | |
"error": err.Error(), | |
"user_error": errorMessage, | |
}) | |
} else { | |
ctx.ServerError("UpdatePullRequestTarget", err) | |
} | |
return |
278d782
to
4ce24ee
Compare
updated tests, now all should PASS :D |
Is this change compitable with Github API? |
What I know: Github provides this feature through it's API. |
git-town/git-town#1518 has gotten merged, git town now supports gitea, but for full support this PR is pressing, everything else is set up. ⌛ The gun is loaded. |
@zeripath You marked this for 1.13.0. Given it's blocking git-town/git-town#1530, any chance to get this shipped earlier? |
@blaggacao as we've already passed the feature freeze, and are now in the RC phase for 1.12 we cannot get it into the 1.12 release. |
I see, ok then, let's head for |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
notwithstanding my comments re:transactions as this doesn't make the function worse I think we can leave that be.
@techknowlogick it might be reasonable to backport this as you could argue it's a bug that the API doesn't have this when the UI does. |
make lg-tm work |
* EditPull: add option to change base Close go-gitea#11552
Backport = #11796 |
* EditPull: add option to change base Close #11552 Co-authored-by: Lauris BH <[email protected]>
Nice, thank you all! |
* EditPull: add option to change base Close go-gitea#11552
Follow go-gitea/gitea#11666 Reviewed-on: https://gitea.com/gitea/go-sdk/pulls/353 Reviewed-by: 6543 <[email protected]> Reviewed-by: John Olheiser <[email protected]>
close #11552