Approvals at Branch Protection #5350

jonasfranz · 2018-11-18T11:43:19Z

This PR adds approvals limitations to branch protection. A pitfall is that it is currently only possible to assign Teams/Users with write permissions to branch protection. I've removed this for teams but not for users.

Screenshots

Signed-off-by: Jonas Franz <[email protected]>

codecov-io · 2018-11-18T12:02:27Z

Codecov Report

Merging #5350 into master will decrease coverage by <.01%.
The diff coverage is 25.47%.

@@            Coverage Diff             @@
##           master    #5350      +/-   ##
==========================================
- Coverage   37.61%   37.61%   -0.01%     
==========================================
  Files         317      318       +1     
  Lines       46835    46928      +93     
==========================================
+ Hits        17619    17650      +31     
- Misses      26712    26770      +58     
- Partials     2504     2508       +4

Impacted Files	Coverage Δ
models/migrations/migrations.go	`2.61% <ø> (ø)`	⬆️
modules/auth/repo_form.go	`39% <ø> (ø)`	⬆️
routers/repo/issue.go	`37.28% <0%> (-0.28%)`	⬇️
models/review.go	`59.77% <0%> (-4.3%)`	⬇️
models/migrations/v74.go	`0% <0%> (ø)`
models/branches.go	`43.33% <25.53%> (-4%)`	⬇️
models/org_team.go	`49.3% <40%> (-0.1%)`	⬇️
routers/repo/setting_protected_branch.go	`40.23% <43.47%> (-0.04%)`	⬇️
models/pull.go	`51.42% <75%> (+0.15%)`	⬆️
... and 2 more

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 64680b7...cf95a76. Read the comment docs.

models/branches.go

lunny · 2018-11-18T13:55:23Z

@JonasFranzDEV any screenshot?

jonasfranz · 2018-11-18T18:55:47Z

@lunny fixed your first comment and added screenshots.

models/review.go

lafriks · 2018-11-21T23:25:47Z

I think this depends on #5132 to have single method that returns latest per-user reviews

lunny · 2018-11-22T01:45:24Z

right, blocked by #5132

lafriks · 2018-11-22T13:22:06Z

Do we allow merging if there is request for changes?

kolaente · 2018-11-22T13:24:10Z

@lafriks I'd make this an option which could be toggled on or off in the settings (default not allowing)

jonasfranz · 2018-11-22T13:25:40Z

@lafriks I think that is out of scope of this PR since this just checks if there are at least X approvals by granted users. But it will only check for the latest approval/rejection of a user as @lunny mentioned.

lafriks · 2018-11-22T13:31:58Z

Yes this can be added in other PR as an option

lunny · 2018-11-22T13:36:42Z

@lafriks I think @JonasFranzDEV is right, allow merging or not when there is request for changes should be another PR.

…re/branch-protection-approvals

…sFranzDEV/gitea into feature/branch-protection-approvals Signed-off-by: Jonas Franz <[email protected]> # Conflicts: # models/review_test.go

Signed-off-by: Jonas Franz <[email protected]>

jonasfranz · 2018-11-28T08:54:42Z

@lunny need your review

lunny · 2018-11-29T12:04:14Z

file conflict.

…re/branch-protection-approvals

jonasfranz · 2018-11-30T17:10:33Z

@lunny File conflicts resolved

lunny · 2018-12-01T06:14:15Z

Pull detail page will result in 500 on MSSQL.

kolaente · 2018-12-02T13:56:51Z

@lunny is this using the function from Review summaries?

lunny · 2018-12-03T01:26:31Z

@kolaente yes, I think that's the reason.

lunny · 2018-12-11T02:03:43Z

Maybe this could be resolved by #5502

jonasfranz · 2018-12-11T07:49:23Z

Can you verify that? @lunny

lunny · 2018-12-11T11:45:18Z

~~@JonasFranzDEV it seems it's has some bug on Mssql. See~~

ghost · 2019-06-25T11:12:50Z

@JonasFranzDEV it seems it's has some bug on Mssql. See

Agree, the same error but with sqlite db.

lunny · 2019-06-26T01:20:25Z

You have to add test1 to approvals list on protected branch. Otherwise his approvals will not be counted.

jonasfranz added 3 commits November 11, 2018 00:33

Add branch protection for approvals

aea75dd

Signed-off-by: Jonas Franz <[email protected]>

Add required approvals

7ab19f5

Signed-off-by: Jonas Franz <[email protected]>

Add missing comments and fmt

2a2dfb5

Signed-off-by: Jonas Franz <[email protected]>

jonasfranz added the type/feature Completely new functionality. Can only be merged if feature freeze is not active. label Nov 18, 2018

jonasfranz added this to the 1.7.0 milestone Nov 18, 2018

Merge branch 'master' into feature/branch-protection-approvals

6a94374

bkcsoft added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Nov 18, 2018

kolaente reviewed Nov 18, 2018

View reviewed changes

models/branches.go Show resolved Hide resolved

lunny requested changes Nov 18, 2018

View reviewed changes

models/branches.go Outdated Show resolved Hide resolved

Add type = approval and group by reviewer_id to review

49092a7

jonasfranz and others added 2 commits November 18, 2018 20:05

Prevent users from adding negative review limits

a983bc7

Merge branch 'master' into feature/branch-protection-approvals

4a2bdd5

lunny reviewed Nov 19, 2018

View reviewed changes

models/review.go Show resolved Hide resolved

kolaente approved these changes Nov 21, 2018

View reviewed changes

bkcsoft 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 Nov 21, 2018

lafriks added the type/changelog Adds the changelog for a new Gitea version label Nov 22, 2018

jonasfranz added 3 commits November 24, 2018 17:05

Merge branch 'master' of https://github.com/go-gitea/gitea into featu…

20d8d9a

…re/branch-protection-approvals

Merge branch 'feature/branch-protection-approvals' of github.com:Jona…

2599e3c

…sFranzDEV/gitea into feature/branch-protection-approvals Signed-off-by: Jonas Franz <[email protected]> # Conflicts: # models/review_test.go

Add migration for approval whitelists

bfe1060

Signed-off-by: Jonas Franz <[email protected]>

lunny mentioned this pull request Nov 26, 2018

Pull request approvals #2417

Closed

lafriks approved these changes Nov 28, 2018

View reviewed changes

bkcsoft 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 Nov 28, 2018

jonasfranz and others added 2 commits November 30, 2018 10:22

Merge branch 'master' of https://github.com/go-gitea/gitea into featu…

025d2f0

…re/branch-protection-approvals

Merge branch 'master' into feature/branch-protection-approvals

ef8d496

Merge branch 'master' into feature/branch-protection-approvals

9e794e2

Merge branch 'master' into feature/branch-protection-approvals

cf95a76

lunny approved these changes Dec 11, 2018

View reviewed changes

lunny merged commit 9681c83 into go-gitea:master Dec 11, 2018

lunny mentioned this pull request Dec 11, 2018

fix approvals limitation #5521

Merged

jonasfranz deleted the feature/branch-protection-approvals branch December 13, 2018 21:13

go-gitea locked and limited conversation to collaborators Nov 24, 2020

delvh removed the type/changelog Adds the changelog for a new Gitea version label Oct 7, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Approvals at Branch Protection #5350

Approvals at Branch Protection #5350

jonasfranz commented Nov 18, 2018 •

edited

Loading

codecov-io commented Nov 18, 2018 •

edited

Loading

lunny commented Nov 18, 2018

jonasfranz commented Nov 18, 2018

lafriks commented Nov 21, 2018

lunny commented Nov 22, 2018

lafriks commented Nov 22, 2018

kolaente commented Nov 22, 2018

jonasfranz commented Nov 22, 2018

lafriks commented Nov 22, 2018

lunny commented Nov 22, 2018

jonasfranz commented Nov 28, 2018

lunny commented Nov 29, 2018

jonasfranz commented Nov 30, 2018

lunny commented Dec 1, 2018

kolaente commented Dec 2, 2018

lunny commented Dec 3, 2018

lunny commented Dec 11, 2018

jonasfranz commented Dec 11, 2018

lunny commented Dec 11, 2018 •

edited

Loading

ghost commented Jun 25, 2019

lunny commented Jun 26, 2019

Approvals at Branch Protection #5350

Approvals at Branch Protection #5350

Conversation

jonasfranz commented Nov 18, 2018 • edited Loading

Screenshots

codecov-io commented Nov 18, 2018 • edited Loading

Codecov Report

lunny commented Nov 18, 2018

jonasfranz commented Nov 18, 2018

lafriks commented Nov 21, 2018

lunny commented Nov 22, 2018

lafriks commented Nov 22, 2018

kolaente commented Nov 22, 2018

jonasfranz commented Nov 22, 2018

lafriks commented Nov 22, 2018

lunny commented Nov 22, 2018

jonasfranz commented Nov 28, 2018

lunny commented Nov 29, 2018

jonasfranz commented Nov 30, 2018

lunny commented Dec 1, 2018

kolaente commented Dec 2, 2018

lunny commented Dec 3, 2018

lunny commented Dec 11, 2018

jonasfranz commented Dec 11, 2018

lunny commented Dec 11, 2018 • edited Loading

ghost commented Jun 25, 2019

lunny commented Jun 26, 2019

jonasfranz commented Nov 18, 2018 •

edited

Loading

codecov-io commented Nov 18, 2018 •

edited

Loading

lunny commented Dec 11, 2018 •

edited

Loading