Re: Process for reviewing patches as someone without commit access

[Christopher Baines <mail@cbaines.net>]

[-- Attachment #1: Type: text/plain, Size: 2478 bytes --]


Christopher Baines <mail@cbaines.net> writes:

> I've been reviewing the list of ideas on and around QA ([1]) recently,
> and got thinking again about how to support people without commit access
> reviewing patches. Obviously you don't need commit access to review
> patches, but where I think we need some process is how to expedite
> someone with commit access taking a look at the patches that have been
> reviewed, and merging them if appropriate.
>
> 1: https://qa.guix.gnu.org/README
>
> Maybe we can use debbugs tags for this? It looks like this has already
> been done in the past, I found some issues tagged with the usertag
> "reviewed" for example [1]. Some were also tagged with
> "reviewed-looks-good". I guess my primary concern is to have a tag (or
> combination of tags) which indicate that a committer should have a look
> at the issue as it's been reviewed by someone and should be ready to
> merge. I don't really use debbugs much, but does anyone have any
> opinions on appropriate tags?
>
> 1: https://debbugs.gnu.org/cgi-bin/pkgreport.cgi?tag=reviewed&users=guix
>
> Once we know what tags to use, I can have the QA frontpage do something
> similar to the "Mark as moreinfo" links, so it's easy to just click a
> button then send the email to change the state of a issue.

I've gone ahead and implemented an initial version of this. On the page
for an issue now there's a form to mark patches as reviewed. This
replaces the previous review checklist and notes field.

That form then takes you to a page to submit the review. This can be
done through the mailto link, or by following the manual
instructions. Of course people using other ways of interacting with
debbugs can also just add the reviewed-looks-good usertag for the guix
user.

Once QA has picked up that the reviewed-looks-good usertag is present,
it'll display a dark green status and the issue will appear at the top
of the /patches page. My hope here is that people with commit access can
prioritise merging patches that have been reviewed and should be ready
to merge.

The implementation is a bit iffy, Mumi doesn't have access to the
usertags as far as I can tell, so I'm getting them from the debbugs soap
interface (luckily Ricardo has already figured that out), but it gives
you every issue tagged for every usertag for that user, which probably
won't scale well if lots of people start reviewing patches. But if that
happens we can always look at how to address it.

Thanks,

Chris

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 987 bytes --]