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.

Sign up for GitHub

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

Jump to bottom

Fix codeowner detected diff base branch to mergebase #29783

Merged
merged 12 commits into from
Mar 15, 2024
Merged

Fix codeowner detected diff base branch to mergebase #29783

merged 12 commits into from
Mar 15, 2024

Conversation

lunny
Copy link
Member

@lunny lunny commented Mar 14, 2024 •
edited
Loading

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
wxiaoguang
wxiaoguang reviewed Mar 14, 2024
View reviewed changes
services/issue/pull.go Outdated Show resolved Hide resolved
wxiaoguang
wxiaoguang reviewed Mar 14, 2024
View reviewed changes
services/issue/pull.go Outdated Show resolved Hide resolved
wxiaoguang
wxiaoguang approved these changes Mar 14, 2024
View reviewed changes
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
@lunny @wxiaoguang
Fix codeowner detected diff base branch to mergebase (go-gitea#29783)
d30950d 
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 <wxiaoguang@gmail.com>
@lunny lunny mentioned this pull request Mar 15, 2024
Merged
@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
@lunny @wxiaoguang
Fix codeowner detected diff base branch to mergebase (#29783) (#29807)
85f31eb 
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 <wxiaoguang@gmail.com>
@lunny lunny mentioned this pull request Mar 27, 2024
Closed
This was referenced Apr 14, 2024
Closed
Merged
silverwind pushed a commit that referenced this pull request Apr 15, 2024
@lunny
Fix code owners will not be mentioned when a pull request comes from …
c63060b 
…a forked repository (#30476)

Fix #30277
Caused by #29783
GiteaBot pushed a commit to GiteaBot/gitea that referenced this pull request Apr 15, 2024
@lunny @GiteaBot
Fix code owners will not be mentioned when a pull request comes from …
e29d1ec 
…a forked repository (go-gitea#30476)

Fix go-gitea#30277
Caused by go-gitea#29783
@GiteaBot GiteaBot mentioned this pull request Apr 15, 2024
Merged
lunny added a commit to lunny/gitea that referenced this pull request Apr 15, 2024
@lunny
Fix code owners will not be mentioned when a pull request comes from …
da35d93 
…a forked repository (go-gitea#30476)

Fix go-gitea#30277
Caused by go-gitea#29783
@lunny lunny mentioned this pull request Apr 15, 2024
Merged
silverwind pushed a commit that referenced this pull request Apr 15, 2024
@GiteaBot @lunny
Fix code owners will not be mentioned when a pull request comes from …
3d31b59 
…a forked repository (#30476) (#30496)

Backport #30476 by @lunny

Fix #30277
Caused by #29783

Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
lunny added a commit that referenced this pull request Apr 16, 2024
@lunny
Fix code owners will not be mentioned when a pull request comes from …
60181eb 
…a forked repository (#30476) (#30497)

Backport #30476 
Fix #30277
Caused by #29783
@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.
Reviewers

@wxiaoguang wxiaoguang wxiaoguang approved these changes

@wolfogre wolfogre wolfogre approved these changes

Assignees
No one assigned
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
Milestone
1.22.0
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
@lunny @silverwind @GiteaBot @wxiaoguang @wolfogre @6543