-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Mark PR reviews as stale at push and allow to dismiss stale approvals #9532
Conversation
Codecov Report
@@ Coverage Diff @@
## master #9532 +/- ##
=========================================
- Coverage 42.17% 42.1% -0.08%
=========================================
Files 583 584 +1
Lines 77360 77485 +125
=========================================
- Hits 32627 32623 -4
- Misses 40721 40850 +129
Partials 4012 4012
Continue to review full report at Codecov.
|
We should save also commit sha on what PR was made so if old commit is reverted back we could remove stale flag |
So the stale flag should be removed if there is a force-push to old commit, or if the old commit is reverted in a new commit, or maybe both? I tested on Github and I don't think they handle that. When a new commit is made Github adds an event:
Would this mean at every new push all old reviews (latest of each reviewer) has to be checked again if they are valid? |
That could be improved in other PRS, more important and quite easy to impelemt that we would set to stale or not based if review commit sha equals currently latest (pushed) |
Later we could improve so that we could check to set stale only if diff actually has changed etc but would be faster to merge this in smaller steps |
The additional code needed to add dismiss stale approvals to branch protection options was small, so I decided to add it in this PR as well. |
# Conflicts: # models/branches.go # models/migrations/migrations.go # models/migrations/v117.go # modules/auth/repo_form.go # routers/repo/setting_protected_branch.go # templates/repo/settings/protected_branch.tmpl
ping lgtm bot |
Fix #5997.
To show that a review is not based on the latest PR changes, an hourglass is shown:
I think there is a point in indicating reviews as being old/stale even if the branch protection does not have the setting to discard stale approvals.
Clean merges should not cause reviews to become stale, as they will not change the diff content of the PR. I have not found any smarter way than generate the diff before and after the push and compare (ignoring some lines). I tested it a bit (and compared to when GitHub marks stale reviews) and it seems to work.
New branch protection option: