Skip to content
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

Fix codeowner detected diff base branch to mergebase #29783

Merged
merged 12 commits into from
Mar 15, 2024

Conversation

lunny
Copy link
Member

@lunny lunny commented Mar 14, 2024

Fix #29763

This PR fix 2 problems about CodeOwner in pull request.

  • Don't use pull request base branch but merge base as a diff base to detect the codeowner.
  • CodeOwner detection in fork repositories will be disabled because almost all the fork repositories will not change CODEOWNERS files but it should not be used on fork repositories' pull requests.

@lunny lunny added type/bug backport/v1.21 This PR should be backported to Gitea 1.21 labels Mar 14, 2024
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Mar 14, 2024
@pull-request-size pull-request-size bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Mar 14, 2024
@github-actions github-actions bot added the modifies/go Pull requests that update Go code label Mar 14, 2024
services/issue/pull.go Outdated Show resolved Hide resolved
services/issue/pull.go Outdated Show resolved Hide resolved
Copy link
Contributor

@wxiaoguang wxiaoguang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall LGTM, approve ahead

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Mar 14, 2024
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Mar 14, 2024
@silverwind
Copy link
Member

Test failure related:

gitea/tests/integration/pull_review_test.go:36
        	Error:      	Received unexpected error:
        	            	initRepository: prepareRepoCommit: GetRepoInitFile[DEFAULT]: file does not exist
        	Test:       	TestPullView_CodeOwner
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
	panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x44b121b]

@lunny
Copy link
Member Author

lunny commented Mar 14, 2024

Test failure related:


gitea/tests/integration/pull_review_test.go:36

        	Error:      	Received unexpected error:

        	            	initRepository: prepareRepoCommit: GetRepoInitFile[DEFAULT]: file does not exist

        	Test:       	TestPullView_CodeOwner

panic: runtime error: invalid memory address or nil pointer dereference [recovered]

	panic: runtime error: invalid memory address or nil pointer dereference

[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x44b121b]

It's very wired. It run successfully local.

@lunny lunny added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Mar 15, 2024
@lunny lunny enabled auto-merge (squash) March 15, 2024 05:04
@lunny lunny merged commit 277f90d into go-gitea:main Mar 15, 2024
26 checks passed
@GiteaBot GiteaBot added this to the 1.23.0 milestone Mar 15, 2024
@GiteaBot
Copy link
Contributor

I was unable to create a backport for 1.21. @lunny, please send one manually. 🍵

go run ./contrib/backport 29783
...  // fix git conflicts if any
go run ./contrib/backport --continue

@GiteaBot GiteaBot added backport/manual No power to the bots! Create your backport yourself! and removed reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. labels Mar 15, 2024
@lunny lunny deleted the lunny/fix_codeowner branch March 15, 2024 06:16
lunny added a commit to lunny/gitea that referenced this pull request Mar 15, 2024
Fix go-gitea#29763

This PR fixes 2 problems with CodeOwner in the pull request.
- Don't use the pull request base branch but merge-base as a diff base to
detect the code owner.
- CodeOwner detection in fork repositories will be disabled because
almost all the fork repositories will not change CODEOWNERS files but it
should not be used on fork repositories' pull requests.

---------

Co-authored-by: wxiaoguang <[email protected]>
@lunny lunny added the backport/done All backports for this PR have been created label Mar 15, 2024
@6543 6543 modified the milestones: 1.23.0, 1.22.0 Mar 16, 2024
6543 pushed a commit that referenced this pull request Mar 17, 2024
Fix #29763
Backport #29783 

This PR fixes 2 problems with CodeOwner in the pull request.
- Don't use the pull request base branch but merge-base as a diff base
to detect the code owner.
- CodeOwner detection in fork repositories will be disabled because
almost all the fork repositories will not change CODEOWNERS files but it
should not be used on fork repositories' pull requests.

---------

Co-authored-by: wxiaoguang <[email protected]>
silverwind pushed a commit that referenced this pull request Apr 15, 2024
GiteaBot pushed a commit to GiteaBot/gitea that referenced this pull request Apr 15, 2024
lunny added a commit to lunny/gitea that referenced this pull request Apr 15, 2024
silverwind pushed a commit that referenced this pull request Apr 15, 2024
…a forked repository (#30476) (#30496)

Backport #30476 by @lunny

Fix #30277
Caused by #29783

Co-authored-by: Lunny Xiao <[email protected]>
lunny added a commit that referenced this pull request Apr 16, 2024
@go-gitea go-gitea locked as resolved and limited conversation to collaborators Jun 13, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backport/done All backports for this PR have been created backport/manual No power to the bots! Create your backport yourself! backport/v1.21 This PR should be backported to Gitea 1.21 lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. modifies/go Pull requests that update Go code size/L Denotes a PR that changes 100-499 lines, ignoring generated files. type/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CODEOWNERS test against diff towards HEAD of target branch instead of merge-base
6 participants