search

coala / meta

A repository for non-code activities, such as engagement initiatives, and other meta issues
6 stars 5 forks source link

How about letting developers ack PRs? #112

Open ishanSrt opened 6 years ago

ishanSrt commented 6 years ago

These PRs can go in a separate queue for maintainers and will definitely not contain trivial errors but may contain design changes. Gitmate can have one more state if PRs are acked by a developer, somewhat like pseudo passed but still not approved for merge.

The more developers ack a PR, the PR can get to one side of the queue, that means it has more probability of being error free. So if someone is not quite in the mood to review, he can start from this end and keep merging.

While those PRs which are acked by lesser developers or (some requested changes and some acked) should go to the other end, as a maintainer will have to look closely and then see why is the PR being controversial.

ishanSrt commented 6 years ago

@sks444 this may concern you

sks444 commented 6 years ago

Thanks, @ishanSrt, we will be doing something like this in the gamification project :) Also, corobo and meta-review project is related to it ;)

Makman2 commented 6 years ago

Not sure if having multiple gitmate states is the right solution (I guess it makes things quite complicated, having different states for different GitHub teams), but in general a good idea :+1:

jayvdb commented 6 years ago

First question is whether gitmate can support it?

If not, this is a dead issue until gitmate supports it.

If you want it, go hack gitmate :P

Actually we have a gitmate plugins project, so this could potentially be done as part of that project.

nkprince007 commented 6 years ago

Uhm, we could change the context & description of the commit status API (both GitHub & GitLab) stating that it was acknowledged / unacknowledged by so and so no. of developers, while still marking it as pending. And we could mark it as approved when a maintainer acks the PR.

Not sure if we could visualize the queue as requested without making some heavy changes to the GitMate.io UI.

ishanSrt commented 6 years ago

Not sure if we could visualize the queue as requested without making some heavy changes to the GitMate.io UI.

maybe just make gitmate set extra tags and labels, which can be filtered in GitHub issue search FTTB.

jayvdb commented 6 years ago

The solution definitely needs to use labels which form the basis of the work queues.

I would be happy enough if developer-acked PRs were given the 'process/approved' label, but the check statuses would still be pending until the maintainer also does an ack. Perhaps rather than allow use of ack, developer might use something else, like pack for 'partial ack', or dack for `developer ack'. Or 'ontrack'...

That puts them into the queue for maintainers to pick them up to do final reviews and merges, and also pushes them out of the review queue, so they wont have lots of other useless "LGTM" comments from developers who are trying to use GitHub as a MMORPG.

jayvdb commented 6 years ago

Another way to do this is replace gitmate ack functionality with the github native review system which now allows a custom list of reviewers. I guess we can set it to require a review from the developers group.

jayvdb commented 6 years ago

I think GitHub requires a CODEOWERS to let developers approve changes.

jayvdb commented 6 years ago

https://github.com/coala/cEPs/issues/122

Also should try on docs repo

jayvdb commented 6 years ago

@ishanSrt , you want to try setting it up for one repo : https://github.com/coala/cEPs/issues/50