Skip to content

Disallow merging if PR has been rejected #4785

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

Closed
FryDay opened this issue Aug 24, 2018 · 11 comments
Closed

Disallow merging if PR has been rejected #4785

FryDay opened this issue Aug 24, 2018 · 11 comments
Labels
issue/duplicate The issue has already been reported.

Comments

@FryDay
Copy link

FryDay commented Aug 24, 2018

If you request changes on a PR, you are still able to merge it. Github disallows merging if a PR is rejected.

@lafriks
Copy link
Member

lafriks commented Aug 24, 2018

Was not there issue already about this?

@FryDay
Copy link
Author

FryDay commented Aug 24, 2018

Not that I was able to find.

@lunny
Copy link
Member

lunny commented Aug 25, 2018

@FryDay I think a PR is rejected by someone could still be merged on Github.

@FryDay
Copy link
Author

FryDay commented Aug 26, 2018

Only someone with admin privileges, and you need to click through warnings to do it.

@lunny
Copy link
Member

lunny commented Aug 27, 2018

@FryDay yes, we could follow that.

@lafriks lafriks added the type/feature Completely new functionality. Can only be merged if feature freeze is not active. label Aug 27, 2018
@stale
Copy link

stale bot commented Jan 12, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs during the next 2 weeks. Thank you for your contributions.

@stale stale bot added the issue/stale label Jan 12, 2019
@stale
Copy link

stale bot commented Feb 21, 2019

This issue has been automatically closed because of inactivity. You can re-open it if needed.

@stale stale bot closed this as completed Feb 21, 2019
@techknowlogick techknowlogick added issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented and removed issue/stale labels Feb 21, 2019
@3l73
Copy link

3l73 commented Apr 15, 2019

AFAIK the reject information stored inside of a review, that is connected to a comment.

In order to disable a pull request, all comments of an issue have to be process, checking if the rejection is still valid.
This solution become a problem depending on the amount of comments.

One possible solution could be changing the relation between comment and review to issue and review.
This solution could lead to

  1. a better runtime (less entries inside a loop)
  2. a listing who reject a pull request
    • this could be used to remove reject status by an administrator
  3. possibility to create a rule that a review is required in order to merge

@3l73
Copy link

3l73 commented Apr 15, 2019

Based on the solution described in the previous comment, there is a way with less refactoring.

The issue is extended with the review, the review remain inside the comment.
The issue indexer process the comments and create the review inside the issue based on the information inside the comment.

Did not know how often the indexer is running and how long it will take to process the data.

@3l73
Copy link

3l73 commented Apr 21, 2019

I will have a look inside this issue.

Other issues going in the same direction:

@lunny
Copy link
Member

lunny commented Sep 12, 2019

duplicated with #3588

@lunny lunny closed this as completed Sep 12, 2019
@lunny lunny added issue/duplicate The issue has already been reported. and removed type/feature Completely new functionality. Can only be merged if feature freeze is not active. issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented labels Sep 12, 2019
@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
issue/duplicate The issue has already been reported.
Projects
None yet
Development

No branches or pull requests

5 participants