guix-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Coordinators for patch review session on Tuesday


From: Steve George
Subject: Re: Coordinators for patch review session on Tuesday
Date: Thu, 4 Apr 2024 15:57:35 +0100

Hi,

Comments below:

On 3 Apr, Christina O'Donnell wrote:
(...)
> Thank you for writing this up in so much depth! I've reviewed [1] and tried
> to tag it as reviewed-looks-good, though I don't think that has gone
> through. If you or someone else could take a look at it then I'd appreciate
> that. I plan on reviewing some more patches this evening.
> 
> Kind regards,
> Christina
> 
> [1] https://debbugs.gnu.org/cgi-bin/bugreport.cgi?users=guix;bug=65938
>

1. Changing the tag to reviewed-looks-good

It doesn't look like this worked. The way to do this is in the instructions are 
4. 'Set a user tag' [0], probably the easiest way is to send an email (I do get 
funny results sometimes with my email client):

Subject: setting usertag on 65938

user guix
usertag 65938 + reviewed-looks-good
quit

The first line is important it has to be 'user guix' for it to appear on the 
patch review reports [1]. I think I messed up the instructions in the Wiki - 
you have to have a + in between the bug number and the tag you want to set 
(sorry about that). Please try again.

This is really just a way of signalling that reviews are happening - so trying 
to keep us in sync. The usertags we're using are:

- patch-review-hackers-list
- under-review
- escalated-review-request 
- waiting-on-contributor
- reviewed-looks-good

The patch changes all look reasonable to me, you've already done a lot:

1. You should add a reviewed-by trailer:
Reviews are contributions from our community (and work!) so we should recognise 
them and add trailers. It also helps the maintainer know who did the review and 
therefore the level of confidence.

Basically just add 'Reviewed-by: A Person <a@email.com> - [2]

It looks like your updated patch retriggered QA, so if you look here and the 
foolow the Data Service link on the right you can see it's building it:

  https://qa.guix.gnu.org/issue/65938

The last step will be for a maintainer to see that it's built correctly, see 
your review and to apply it - great job for a first patch review!

Steve / Futurile

[0] 
https://libreplanet.org/wiki/Group:Guix/PatchReviewSessions2024#Patch_review_process_-_CLI_tools
[1] 
https://libreplanet.org/wiki/Group:Guix/PatchReviewSessions2024#Patch_review_states_and_reports
[2] 
https://libreplanet.org/wiki/Group:Guix/PatchReviewSessions2024#10._Add_a_Reviewed-by_Trailer



reply via email to

[Prev in Thread] Current Thread [Next in Thread]