I would like to post a feature request for MR which seems related to this. Can you elaborate on what you mean by "voting"? My feature request is pretty much:
Allow more than one (or more than "require this many approvals") users to approve an MR. Sometimes we want two or three users to sign off on an MR, but that's not required for it to be merged, it's just a convention.
Allow users to take back their approvals (unapprove).
Allow users to approve even if the number of required approvals has been reached.
Voting sounds like it would exactly fit this scenario, so, roughly, there would be an "approve/unapprove" toggle that would be available for everyone, regardless of how many other approvals there are, and a "merge" button that would only be available after N approvals have been reached.
Does this sound like something you are planning, or should I file another issue?
EDIT: I believe this is exactly how Stash (and possibly BitBucket?) does things now.
@regisF did you mean to close this? It's still open
While I've got your attention, I wonder if we should just update the issue body and try to prioritise some of these open issues for approvals. I know there are some dupes and they aren't all labelled properly, but at least they are here!
Issues about discovering approvals needed / completed:
Store merge request approvers in the repository instead of in the settings: #510 (not sure if we should do this, the repository doesn't know about users)
@stavros I think your unapproval scenario is covered in the issues above. For approvals over the number needed, I think that's an interesting idea and I don't think it's in any of the above issues (I might have misremembered). Please create a separate issue for that and link it to this one if it's something you're still interested in
@awhildy + @smcgivern : I did some house cleaning in regards to approvals. You can review the description at the top to see features that have been done, features already in flight, and future ideas to be worked on.
Regarding 8.15, we have our first iteration of unapproving (https://gitlab.com/gitlab-org/gitlab-ee/issues/894), which is a fairly major change, since it changes system state. I expect to get a lot of feedback from that.
How about for 8.15, we also work on enhancing the list view of merge request to:
Allow a user to glance at approver info in each item as they scroll through the list.
Filter the list based on approver info.
I think this is a good balance because:
We are not introducing another feature that changes system state. (https://gitlab.com/gitlab-org/gitlab-ee/issues/894 already does that in regards to approvals for this milestone.) This is already disruptive for the user and the codebase. "Disruptive" in the sense of high risk and high reward.
@victorwu thanks! @jneen will be working on the backend for approvals improvements for 8.15. Please add issues to the milestone as they're ready for backend work, and she'll pick them up.
@smcgivern : I added two additional issues for 8.15. One is for the system note to be shown in the merge request discussion thread. And the other is the notification email. Do you know any other areas we are missing this type of action?
@smcgivern : Also I'm able to set up EE on my local machine. But I'm not able to figure out just yet how to get a license on it, set up fake users, generate emails, and such. So I don't have all the edge cases covered. But I imagine our system note and email conditions for removing an approval should be the same as approving a merge request. So that should be enough specification for now before @jneen takes a look and gives us some feedback on what would be good.
@awhildy : When @jneen starts picking up those backend tasks, could you provide some guidance on the text copy, especially the system notes and the email notifications. I'm not able to reproduce the situations on my machine or a fake project to add any screenshots. But just reading through the code, it looks like the email notification text is in passive voice, whereas the system not is in an active voice. Feel free to suggest any changes if you think those are necessary. We might as well do it now, since we are digging into this code now. The code is linked in the issues so you can take a look.
Sounds good @victorwu. The suggestions you had for the system note make sense to me, and I suspect we can just swap approved to unapproved in the email (as it is all past tense), but there might be more copy that I'm missing to consider. @jneen - when you start digging into this, or if there is anything I can help out with, let me know. Thanks much!