Skip to content

Allow repo admin to merge PR regardless of review status #9611

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
Merged
techknowlogick merged 17 commits into from
Jan 11, 2020
Merged

Allow repo admin to merge PR regardless of review status #9611

techknowlogick merged 17 commits into from
Jan 11, 2020

Conversation

@davidsvantesson
Copy link
Contributor

@davidsvantesson davidsvantesson commented Jan 5, 2020

  • Allow repo admin to merge even if not all reviews are done/ok. Currently only status checks was possible to bypass
  • Show red button instead of green for admin in this case (see screeshot)
  • API merge endpoint add optional force_merge option which corresponds to above (only for admin)
  • Some refactoring and fixes

Screenshot

image

@lafriks lafriks added the type/enhancement An improvement of existing functionality label Jan 5, 2020
@lafriks lafriks added this to the 1.12.0 milestone Jan 5, 2020
@lafriks lafriks added the type/changelog Adds the changelog for a new Gitea version label Jan 5, 2020
@davidsvantesson davidsvantesson changed the title Repo admin merge pr Allow repo admin to merge PR regardless of review status Jan 5, 2020
lafriks
lafriks reviewed Jan 6, 2020
View reviewed changes
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jan 6, 2020
lafriks
lafriks reviewed Jan 6, 2020
View reviewed changes
@codecov-io
Copy link

codecov-io commented Jan 6, 2020 •
edited
Loading

Codecov Report

Merging #9611 into master will increase coverage by <.01%.
The diff coverage is 88.88%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #9611      +/-   ##
==========================================
+ Coverage   42.21%   42.21%   +<.01%     
==========================================
  Files         590      591       +1     
  Lines       78085    78085              
==========================================
+ Hits        32961    32964       +3     
+ Misses      41077    41075       -2     
+ Partials     4047     4046       -1
Impacted Files Coverage Δ
models/issue_tracked_time.go 58.67% <ø> (-4.18%) ⬇️
modules/convert/issue.go 100% <100%> (ø)
routers/api/v1/repo/issue_tracked_time.go 39.56% <40%> (ø) ⬆️
modules/process/manager.go 78.31% <0%> (+3.61%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 618e5e3...1b75cda. Read the comment docs.

zeripath
zeripath reviewed Jan 6, 2020
View reviewed changes
zeripath
zeripath reviewed Jan 6, 2020
View reviewed changes
zeripath
zeripath reviewed Jan 6, 2020
View reviewed changes
zeripath
zeripath reviewed Jan 6, 2020
View reviewed changes
@lafriks
Copy link
Member

lafriks commented Jan 7, 2020

@zeripath that is not correct English

@guillep2k
Copy link
Member

guillep2k commented Jan 7, 2020

@zeripath that is not correct English

@lafriks You mean cannot?

@davidsvantesson
Copy link
Contributor Author

davidsvantesson commented Jan 7, 2020

I reformulated the messages instead 😏

guillep2k
guillep2k reviewed Jan 8, 2020
View reviewed changes
bagasme
bagasme reviewed Jan 8, 2020
View reviewed changes
lunny
lunny requested changes Jan 8, 2020
View reviewed changes
@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 Jan 9, 2020
@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 Jan 9, 2020
@davidsvantesson davidsvantesson requested a review from lunny January 9, 2020 17:44
lunny
lunny reviewed Jan 10, 2020
View reviewed changes
services/pull/merge.go
}
prConfig := prUnit.PullRequestsConfig()

if err := pr.CheckUserAllowedToMerge(doer); err != nil {
Copy link
Member

@lunny lunny Jan 10, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

So all permissions check are out of this function?

@techknowlogick techknowlogick merged commit 32fb813 into go-gitea:master Jan 11, 2020
@davidsvantesson davidsvantesson deleted the repo-admin-merge-pr branch January 11, 2020 10:27
@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
@delvh delvh removed the type/changelog Adds the changelog for a new Gitea version label Oct 7, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@lunny lunny lunny approved these changes

@lafriks lafriks lafriks approved these changes

@6543 6543 6543 approved these changes

+3 more reviewers

@zeripath zeripath zeripath left review comments

@guillep2k guillep2k guillep2k left review comments

@bagasme bagasme bagasme left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/enhancement An improvement of existing functionality

Projects

None yet

Milestone

1.12.0

Development

Successfully merging this pull request may close these issues.