Pull Request Pusher should be the author of the merge#36581
Merged
lunny merged 11 commits intogo-gitea:mainfrom Mar 2, 2026
Merged
Pull Request Pusher should be the author of the merge#36581lunny merged 11 commits intogo-gitea:mainfrom
lunny merged 11 commits intogo-gitea:mainfrom
Conversation
GiteaBot
added
the
lgtm/need 2
AdamMajer
force-pushed
the
pushes-is-author
branch
from
2df61c3 to
55ae210
Compare
Contributor
Author
|
It's not exactly clear that the pusher shouldn't always be considered the merger. For example, if someone merges manually one branch into another, but 3rd party is pushing this change to the target repo, it would make sense that the pusher is always the actual merger for the repository when it comes to manual merge detection. Should I just change the default behaviour always? |
Member
|
Yes, I believe the pusher should generally be regarded as the merger. |
Contributor
There was a problem hiding this comment.
Pull request overview
Updates manual-merge autodetection to attribute the merger correctly for fast-forward-only merges by using the branch pusher (while keeping existing behavior for merge commits), and adds an integration test to cover the new behavior.
Changes:
- Adjust
manuallyMergedmerger resolution: merge commits use commit author email; fast-forward merges use base-branch pusher. - Add an integration test that performs a fast-forward merge via
git merge+git pushand assertsMergerIDequals the pusher.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
services/pull/check.go |
Changes merger attribution logic for ff-only manual merges to use the recorded branch pusher. |
tests/integration/manual_merge_test.go |
New integration test validating merger attribution for fast-forward manual merges. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Member
|
Please find a more descriptive title for this PR. |
AdamMajer
changed the title
AdamMajer
changed the title
Member
|
Please address or resolve above two copilot comments. |
Contributor
Author
|
Let's change this to always use the |
In manual merge detected changes, the pushing user should be the de-facto author of the merge, not the committer. For ff-only merges, the author (PR owner) often have nothing to do with the merger. Similarly, even if a merge commit exists, it does not indicate that the merge commit author is the merger. If pusher is for some reason unavailable, we fall back to the old method of using committer or owning organization as the author.
AdamMajer
force-pushed
the
pushes-is-author
branch
from
34821d5 to
6317ddb
Compare
wxiaoguang
reviewed
Mar 2, 2026
wxiaoguang
force-pushed
the
pushes-is-author
branch
from
4a70676 to
97da5ce
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
|
Does the new change look good to you? Or anything else to address? |
AdamMajer
commented
Mar 2, 2026
Contributor
Author
Aside form my general question about user ==nil for ghost users, it looks good. |
wxiaoguang
approved these changes
Mar 2, 2026
GiteaBot
added
lgtm/need 1
lunny
approved these changes
Mar 2, 2026
GiteaBot
added
lgtm/done
lunny
added
reviewed/wait-merge
GiteaBot
removed
the
reviewed/wait-merge
zjjhot
added a commit
to zjjhot/gitea
that referenced
this pull request
Mar 4, 2026
* giteaofficial/main:
[skip ci] Updated translations via Crowdin
fix: /repos/{owner}/{repo}/actions/{runs,jobs} requiring owner permissions (go-gitea#36818)
Fix CRAN package version validation to allow more than 4 version components (go-gitea#36813)
Fix API not persisting pull request unit config when has_pull_requests is not set (go-gitea#36718)
feat: Add Actions API rerun endpoints for runs and jobs (go-gitea#36768)
Fix bug when pushing mirror with wiki (go-gitea#36795)
Pull Request Pusher should be the author of the merge (go-gitea#36581)
Delete non-exist branch should return 404 (go-gitea#36694)
Remove API registration-token (go-gitea#36801)
Add background and run count to actions list page (go-gitea#36707)
silverwind
added a commit
to silverwind/gitea
that referenced
this pull request
Mar 6, 2026
* origin/main: (27 commits) Fix OAuth2 authorization code expiry and reuse handling (go-gitea#36797) Fix org permission API visibility checks for hidden members and private orgs (go-gitea#36798) Fix non-admins unable to automerge PRs from forks (go-gitea#36833) upgrade to github.com/cloudflare/circl 1.6.3, svgo 4.0.1, markdownlint-cli 0.48.0 (go-gitea#36837) Fix dump release asset bug (go-gitea#36799) build(deps): update material-icon-theme v5.32.0 (go-gitea#36832) Fix bug to check whether user can update pull request branch or rebase branch (go-gitea#36465) Fix forwarded proto handling for public URL detection (go-gitea#36810) Fix artifacts v4 backend upload problems (go-gitea#36805) Add a git grep search timeout (go-gitea#36809) fix(repo): unify DEFAULT_SHOW_FULL_NAME output in templates and dropdown (go-gitea#36597) Harden render iframe open-link handling (go-gitea#36811) [skip ci] Updated translations via Crowdin fix: /repos/{owner}/{repo}/actions/{runs,jobs} requiring owner permissions (go-gitea#36818) Fix CRAN package version validation to allow more than 4 version components (go-gitea#36813) Fix API not persisting pull request unit config when has_pull_requests is not set (go-gitea#36718) feat: Add Actions API rerun endpoints for runs and jobs (go-gitea#36768) Fix bug when pushing mirror with wiki (go-gitea#36795) Pull Request Pusher should be the author of the merge (go-gitea#36581) Delete non-exist branch should return 404 (go-gitea#36694) ... # Conflicts: # routers/web/repo/issue_view.go
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
In manual merge detected changes, the pushing user should be the de-facto author of the merge, not the committer. For ff-only merges, the author (PR owner) often have nothing to do with the merger. Similarly, even if a merge commit exists, it does not indicate that the merge commit author is the merger. This is especially true if the merge commit is a ff-only merge on a given branch.
If pusher is for some reason unavailable, we fall back to the old method of using committer or owning organization as the author.