search

go-gitea / gitea

Git with a cup of tea! Painless self-hosted all-in-one software development service, including Git hosting, code review, team collaboration, package registry and CI/CD
https://gitea.com
MIT License
43.82k stars 5.38k forks source link

PRs that have open dependencies are not closed automatically when merged locally #14444

Open MathJoDE opened 3 years ago

MathJoDE commented 3 years ago

Description (steps to reproduce)

  1. create a PR and an issue
  2. add the issue as depenency to the PR
  3. merge the branch locally into master, then git push

Result: a) PR still open in gitea b) the commit and changed files tabs are "N/A"

(If you want to go further:

  1. close the dependency issue
  2. merge the PR over the UI --> 500 internal server error, but the PR is closed now.)

Screenshots

I'll add one if adding dependencies on try.gitea.io works

MathJoDE commented 3 years ago

Edit: added the internal system information of our gitea server (although I guess they have no impact on this issue).

delvh commented 3 years ago

Well, is that really a bug? The point of PRs is that it should not be merged locally, but remotely (after having given others the option to review it). If you open a PR, you normally expect it to be answered using some kind of UI on the git hosting system. If you locally merge a branch, not only do you ignore the purpose of PRs, no, you go beyond what Gitea's PRs are designed for. Pull requests are not a built-in feature of git, they are provided by the git hosting system like GitHub, GitLab or Gitea. If you manually override them, do as you like, but I'd argue that it is in that case your own responsibility to clean up behind yourself.

MathJoDE commented 3 years ago

I'm not a professional git developer, so I can't judge whether this is abuse of PRs or not. However, an advantage of merging locally I experienced is that Gitea sometimes sees conflicts (and thus prevents merging), while git locally does resolve these conflicts automatically without requiring manual intervention.

Independently of that, Gitea (try.gitea.io) has even a prominently placed guide how to merge PRs locally. Screenshot_20210124_163337

delvh commented 3 years ago

Well, let's consider the following case that can be encountered in most bigger projects: You open a PR in branch test for branch master. Master has branch-protection strictly turned on, meaning that no one can commit anything there - PRs are excluded from that. You have a merge conflict between test and master, so you merge locally as you suggested. How do you want to push the local master into the remote master? As intended, it is impossible to commit anything into the master branch. Now your method fails, as expected.

The correct approach would be to commit the merge from master into your branch test and push that commit, where you can then merge the PR.

zeripath commented 3 years ago

have you reported this: " No, since currently adding dependencies whyever does not work there. (Current gitea version on try.gitea.io: 1.14.0+dev-598-g4acb499f3)."

MathJoDE commented 3 years ago

have you reported this: " No, since currently adding dependencies whyever does not work there. (Current gitea version on try.gitea.io: 1.14.0+dev-598-g4acb499f3)."

Done. #14449

septatrix commented 2 years ago

Another reason for merging locally is the ability to use different merge algorithms and drivers

InExtremaRes commented 3 months ago

There is yet another reason for merging locally: if you want every single commit signed with a key you personally trust.

It's true that you can configure the internal Gitea user to sign its own commits and you can import and trust that public key to your local GPG keyring, but maybe you don't want to expose a commit made by "Gitea \gitea@fake.local" or you don't want to trust its public key. Even more, maybe you don't have control over the gitea instance and the Gitea user doesn't have a public key and cannot sign the merge commit itself; if you have a branch protection in place, then gitea will refuse to merge since it can't sign, leaving you with the only option of merging locally and sign yourself.