Allow Collaborators with Write Access to Give Approving PR Reviews

[bagasme]

• Gitea version (or commit ref): 1.9.0
• Git version: 2.17.1
• Operating system: Ubuntu 18.04
• Database (use [x]):
  • PostgreSQL
  • MySQL
  • MSSQL
  • SQLite
• Can you reproduce the bug at https://try.gitea.io:
  • Yes (provide example URL)
  • No
  • Not relevant
• Log gist:
  (not relevant)

Description

Currently in Gitea, merging PRs against protected branch with required reviews can only be allowed with enough approving reviews from whitelisted reviewers. However, on GitHub, approving reviews can be given from users with write (or admin) access to the repo. If this feature will be implemented to Gitea, whitelisted reviewers list should contain additional users that can give approving reviews.

The caveat is, when a collaborator with write, admin, or owner access submit request change review, the PR cannot be merged until the same collaborator submits another approving reviews.

Thus, the wording for Review Required message should be (when no approvals yet):

This Pull Request doesn't have approvals yet. 0 of 1 approvals granted from reviewers with write access and whitelisted reviewers.

Screenshots

Review Required message on a PR in Gitea:

On GitHub:

[lunny]

You can customize that on protected branches

[guillep2k]

@lunny Approvals from users not listed in the whitelist are currently ignored. I think the proposal here is to count approvals from users that are not in the whitelist, but have write access.

User "prueba" has write access, and approved the changes, but the merge is still disabled because "prueba" is not whitelisted.

[bagasme]

@guillep2k:

I think the proposal here is to count approvals from users that are not in the whitelist, but have write access.

Thus, the whitelist can contain additional users that are eligible for approvals.

I mean, the approvals are granted from users with write access AND whitelisted users, that is regardless of whether the user with write access is whitelisted or not, he/she can give approving reviews.

[davidsvantesson]

I don't think you necessary want all "writers" to be reviewers. If you have a workflow where changes are done in branches directly in the repo (rather than the fork-pull request workflow) there might only be certain that are allowed to review.
Maybe there can be always a 'Write access' team possible to add?

[MarkusAmshove]

For our workflow (no forks, protected master branch) this would also be a drawback, as mentioned by @davidsvantesson

[lunny]

A user has write permission will not be allowed to push to protected branch directly, but you want he can always approve the pull requests? I don't think that's reasonable. If you want to do that, add him to the whitelist. And if you have many users and don't want to maintain a whitelist, maybe you could consider to transfer the repository to an organization and add a team to the whitelist.

[guillep2k]

@davidsvantesson @lunny I see your point.

[bagasme]

Here on GitHub, instead of using whitelists, PR reviews can be given from writers (and admins). But why Gitea choose whitelists approach?

[davidsvantesson]

A problem with current implementation is if you don't put any team or user in whitelist reviewers there is nobody that can do review. In my opinion anybody with access to the repo should be able to do review in that case (readers or writers).

@bagasme This solution gives more fine-grained options which ones is allowed to do reviews. It is possible to achieve the same as GitHub if you give a team access to the repo and also put the same team in the whitelist.

[bagasme]

@davidsvantesson:

In my opinion anybody with access to the repo should be able to do review in that case (readers or writers).

You mean anyone with access with access to the repo (at least readers) can give comment reviews, right?

[davidsvantesson]

My point is there is no point in having required approvals without anyone being able to approve. So the default behavior if no white-listed user or team has been added could be that anyone with write access could review and approve (which would match GitHub according to what you wrote).

I think anyone already can make review comments (but it will not count for approvals).

[guillep2k]

What about a checkbox? "Anyone with write access"; if it's checked, the white list is grayed out.

[bagasme]

@guillep2k There is it. Repo admins can choose who reviewers can be counted for approval either writers (default on GitHub) or whitelisted users (currently on Gitea, if admins are a little paranoid about security).

[davidsvantesson]

@guillep2k Wouldn't it suffice with updating the help text and that would be the default if no whitelist is given?

@bagasme I don't think it is about being paranoid. You simply have specific users that do reviews (could even be users which doesn't have write access).