No way to require Approval from specific person or team

Bug #513176 reported by Stuart Metcalfe
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Tarmac
Triaged
Medium
Unassigned

Bug Description

It would be great to have customisable rules for when merges can be made. For example:

To ensure that, in addition to the regular code reviewer, a specific team (maybe a QA team) has approved the merge (by testing the proposed code) before it is able to be merged.

Tags: plugin
Paul Hummer (rockstar)
Changed in tarmac:
status: New → Triaged
importance: Undecided → High
assignee: nobody → Paul Hummer (rockstar)
milestone: none → tarmac-0.3
Revision history for this message
Paul Hummer (rockstar) wrote :

I think this should be fixed about the same time as bug #426950

Changed in tarmac:
importance: High → Wishlist
assignee: Paul Hummer (rockstar) → nobody
milestone: tarmac-0.3 → none
tags: added: plugin
Revision history for this message
Paul Hummer (rockstar) wrote :

Stuart, can you give me a use case for this? Is there something outside of using isPersonTrustedReviewer to check that the reviewer was in a certain team that you want to do?

This might be a dupe of bug #397868 and/or bug #426950

Changed in tarmac:
milestone: none → tarmac-0.4
Paul Hummer (rockstar)
Changed in tarmac:
milestone: tarmac-0.4 → none
Revision history for this message
dobey (dobey) wrote :

It sounds like he wants to be able to specify that one of the Approved votes is from someone in a specific subset of the people who are in the isPersonTrustedReviewer() list. Such as having a -qa sub-team from which someone must have Approved, before it can be merged.

Does that sound right Stuart?

Revision history for this message
Stuart Metcalfe (stuartmetcalfe) wrote :

Rodney, that's right. I suspect bug #426950 will do this for us.

Revision history for this message
dobey (dobey) wrote :

No, resolving that bug won't solve that issue as well. That is simply about moving the current usage of isPersonTrustedReviewer (which is probably not especially great at the moment anyway), into a plug-in that can be disabled.

With that, bug #633939 and bug #426950 are more along the same lines of what they should do. I think simply moving the isPersonTrustedReviewer check into the votes plug-in, and simply recording all the reviewers in the metadata we stick in the new revision, will be ideal, and resolve those two bugs.

I think for your case (this bug), we'll probably want another plug-in, or another extension to the current votes plug-in to also require one or more approvals from a list of persons or teams. With the votes plug-in, this could be an additional config option, or perhaps an extension to the voting_criteria parsing to allow specifying a person or team, sort of like: Approved >= 2, Approved (qa-team) >= 1

dobey (dobey)
summary: - Enable rules for merging
+ No way to require Approval from specific person or team
Changed in tarmac:
importance: Wishlist → Medium
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.