Approvals are dismissed when new commits are pushed #11670

New Issue

GiteaMirror · 2025-11-02T09:44:10-06:00

GiteaMirror commented

2025-11-02 09:44:10 -06:00

Originally created by @jpraet on GitHub (Sep 18, 2023).

Description

With https://github.com/go-gitea/gitea/pull/25882, if

is set on the branch protection, approvals get dismissed when new commits are added.

I am not sure if it was the actual intent of this branch protection to actively "Dismiss" the review.

Previously, this option already caused stale approvals to be ignored towards the PR approval count, which seems sufficient?

a50d9af876/models/issues/pull.go (L837-L839)

Now, the review is actually dismissed, resulting in an entry in the PR history, and an (empty) email notification.

This dismissed review is also not shown in the "Reviewers" view of the PR, whereas stale reviews are.

In my opinion actually dismissing the review because new commits are added is a bit excessive?

Gitea Version

1.20.4

Can you reproduce the bug on the Gitea demo site?

Yes

Log Gist

No response

Screenshots

No response

Git Version

No response

Operating System

No response

How are you running Gitea?

docker

Database

None

Originally created by @jpraet on GitHub (Sep 18, 2023). ### Description With https://github.com/go-gitea/gitea/pull/25882, if ![image](https://github.com/go-gitea/gitea/assets/2212615/5752cbc9-6023-4b30-b407-c9dfaa6bec97) is set on the branch protection, approvals get dismissed when new commits are added. I am not sure if it was the actual intent of this branch protection to actively "Dismiss" the review. Previously, this option already caused stale approvals to be ignored towards the PR approval count, which seems sufficient? https://github.com/go-gitea/gitea/blob/a50d9af876435d007e6052c6ef8ebc838dd9709f/models/issues/pull.go#L837-L839 Now, the review is actually dismissed, resulting in an entry in the PR history, and an ([empty](https://github.com/go-gitea/gitea/issues/27035)) email notification. This dismissed review is also not shown in the "Reviewers" view of the PR, whereas stale reviews are. In my opinion actually dismissing the review because new commits are added is a bit excessive? ### Gitea Version 1.20.4 ### Can you reproduce the bug on the Gitea demo site? Yes ### Log Gist _No response_ ### Screenshots _No response_ ### Git Version _No response_ ### Operating System _No response_ ### How are you running Gitea? docker ### Database None

GiteaMirror added the type/bug label 2025-11-02 09:44:10 -06:00

GiteaMirror closed this issue

2025-11-02 09:44:10 -06:00

GiteaMirror commented

2025-11-02 09:44:11 -06:00

@jpraet commented on GitHub (Sep 18, 2023):

The PR https://github.com/go-gitea/gitea/pull/9532 to mark PR reviews as stale at push and allow to dismiss stale approvals was merged in 2020.
The PR https://github.com/go-gitea/gitea/pull/12674 to add dismiss review feature was merged in 2021.

So I believe the term "dismiss" in the branch protection setting does not actually refer to the real "dismiss review" functionality because that didn't exist yet.

@jpraet commented on GitHub (Sep 18, 2023): The PR https://github.com/go-gitea/gitea/pull/9532 to mark PR reviews as stale at push and allow to dismiss stale approvals was merged in 2020. The PR https://github.com/go-gitea/gitea/pull/12674 to add dismiss review feature was merged in 2021. So I believe the term "dismiss" in the branch protection setting does not actually refer to the real "dismiss review" functionality because that didn't exist yet.

GiteaMirror commented

2025-11-02 09:44:11 -06:00

@lunny commented on GitHub (Sep 28, 2023):

#25882 is to fix #25858 . So maybe the definition of Dismiss stale approvals is no very clear. The previous implementation will not update the dismissed column but stale column and it will be ignored when counting official approvals. But the UI is still not right.

@lunny commented on GitHub (Sep 28, 2023): #25882 is to fix #25858 . So maybe the definition of `Dismiss stale approvals` is no very clear. The previous implementation will not update the dismissed column but stale column and it will be ignored when counting official approvals. But the UI is still not right.

GiteaMirror commented

2025-11-02 09:44:11 -06:00

@jpraet commented on GitHub (Sep 28, 2023):

Yes, in my opinion it would have been better to update the UI description (e.g. "Ignore stale approvals") of this branch protection setting to match the implemented behavior. Instead of updating the implementation to match the description.

@jpraet commented on GitHub (Sep 28, 2023): Yes, in my opinion it would have been better to update the UI description (e.g. "Ignore stale approvals") of this branch protection setting to match the implemented behavior. Instead of updating the implementation to match the description.

GiteaMirror commented

2025-11-02 09:44:11 -06:00

@jpraet commented on GitHub (Dec 5, 2023):

Dismissing the stale approval reviews also has the following unwanted side effects:

if a review was requested from the author of the stale approval, it does not revert to the "review requested" state
it is not possible for the PR author to re-request a review from the author of the stale approval

@jpraet commented on GitHub (Dec 5, 2023): Dismissing the stale approval reviews also has the following unwanted side effects: * if a review was requested from the author of the stale approval, it does not revert to the "review requested" state * it is not possible for the PR author to re-request a review from the author of the stale approval

GiteaMirror commented

2025-11-02 09:44:11 -06:00

@wderoey commented on GitHub (Dec 5, 2023):

Agreeing with @jpraet ; simply ignoring a stale approval suffices. Dismissing a review completely when new commits are added is not a behavior you wish to make automatic. So i am pro updating the UI description to "Ignore stale approvals" and reverting the implemented behaviour to update the stale column and ignore the approval when counting official approvals (not updating the dismiss column).

@wderoey commented on GitHub (Dec 5, 2023): Agreeing with @jpraet ; simply ignoring a stale approval suffices. Dismissing a review completely when new commits are added is not a behavior you wish to make automatic. So i am pro updating the UI description to "Ignore stale approvals" and reverting the implemented behaviour to update the stale column and ignore the approval when counting official approvals (not updating the dismiss column).

GiteaMirror commented

2025-11-02 09:44:12 -06:00

@jpraet commented on GitHub (Dec 17, 2023):

Would a PR that reverts to the old behavior and clarifies the UI description be accepted?

Or would a PR that adds a new "Ignore stale approvals" branch protection setting be preferred? That way, both options are supported.

@jpraet commented on GitHub (Dec 17, 2023): Would a PR that reverts to the old behavior and clarifies the UI description be accepted? Or would a PR that adds a new "Ignore stale approvals" branch protection setting be preferred? That way, both options are supported.

GiteaMirror commented

2025-11-02 09:44:12 -06:00

@lunny commented on GitHub (Dec 17, 2023):

I would like the second option.

@lunny commented on GitHub (Dec 17, 2023): I would like the second option.

GiteaMirror commented

2025-11-02 09:44:12 -06:00

@github-actions[bot] commented on GitHub (Mar 1, 2024):

Automatically locked because of our CONTRIBUTING guidelines

@github-actions[bot] commented on GitHub (Mar 1, 2024): Automatically locked because of our [CONTRIBUTING guidelines](https://github.com/go-gitea/gitea/blob/main/CONTRIBUTING.md#issue-locking)

Sign in to join this conversation.

Branches Tags

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: github-starred/gitea#11670