[webkit-dev] Approving / Rejects PRs on GitHub when not a reviewer?

Chris Dumez cdumez at apple.com
Tue Nov 28 16:12:42 PST 2023


> On Nov 28, 2023, at 4:06 PM, Jean-Yves Avenard <jean-yves.avenard at apple.com> wrote:
> 
> Hi
> 
>> On 29 Nov 2023, at 9:44 am, Chris Dumez via webkit-dev <webkit-dev at lists.webkit.org> wrote:
>> 
>> FYI, our official documentation on WebKit.org <http://webkit.org/> says:
>> ```
>> Making unofficial reviews before you become a reviewer is encouraged. This is an excellent way to show your skills. Note that you should not put r+ nor r- on patches in such unofficial reviews.
>> ```
>> I guess this wan’t updated after the move to GitHub. For me, no r+ or r- on bugzilla translates to no approve / deny PRs on GitHub. So I simply wish we’d start enforcing this policy again.
>> 
>> Having the tools help us would be great but I don’t think it stops us from enforcing our own policies like we used to.
> 
> Personally, I’ve been requesting non-official reviewers to review my patches because I know that their skill set is perfectly matched (and it will help make them official reviewer)
> 
> Having them giving r+ explicitly is, I find, easier to spot than looking through the often busy GitHub page to find the comments.

Even if someone approves, you’d still need to find the comments. r+ with comments is super common.

> 
> Could we relax the ability to give informal r+ review to people with commit rights? 

They can just leave a comment saying the patch looks good. They don’t have to approve the PR. Approving the PR means nothing since it won’t let you merge your PR.
People are expected to wait for an official review to trigger the merge queue, we’re currently making it harder than it needs to be if we have an official review or not.

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-dev/attachments/20231128/6ac49852/attachment.htm>


More information about the webkit-dev mailing list