On Tue, Nov 28 2023 at 10:27:41 AM -0800, Chris Dumez via webkit-dev <webkit-dev@lists.webkit.org> wrote:
The reason I would like us to make enforce this rule is that I find it confusing. We have a lot of new comers in the project and I do not always know if a person is a reviewer or not yet. I imagine it may be even more confusing for non-Apple people.
I have in some cases not reviewed patches because I had seen the "green check” and thought the PR already had been approved.
+1, if there is a green checkmark, then I assume that person is a reviewer. It's really confusing when non-reviewers add the green checkmark.
I have also seen cases of PRs rejected, asking the author to do more work, that I didn’t feel was necessary.
Hm, I wonder if these situations were exceptions rather than common situations? In general, I would expect that if a non-reviewer is requesting changes on a pull request, they are *probably* justified in doing so, so I'm much less concerned about non-reviewer use of "Request changes" than I am about non-reviewer use of "Approved." The submitter can always argue against the requests, after all. It's very common for a reviewer to request something and for the change author to push back. We usually agree in the end. :) I don't have a strong opinion on this. Michael