From: Steve George <steve@futurile.net>
To: Christina O'Donnell <cdo@mutix.org>
Cc: guix-devel <guix-devel@gnu.org>, efraim@flashner.co.il
Subject: Re: Coordinators for patch review session on Tuesday
Date: Thu, 4 Apr 2024 15:57:35 +0100 [thread overview]
Message-ID: <bkhdx6fcofvnwlvihx2n3zfq5denl756fdwzihlvgjb6ebnwyi@27jgemfrz7oo> (raw)
In-Reply-To: <55f9be63-419e-57ba-7bd8-691cf80ab012@mutix.org>
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
next prev parent reply other threads:[~2024-04-04 14:58 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-31 21:58 Coordinators for patch review session on Tuesday Steve George
2024-04-02 10:09 ` Christina O'Donnell
2024-04-02 20:23 ` Steve George
2024-04-03 11:00 ` Christina O'Donnell
2024-04-03 23:01 ` Christina O'Donnell
2024-04-04 14:57 ` Steve George [this message]
2024-04-04 17:27 ` Christina O'Donnell
2024-04-05 7:54 ` Steve George
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://guix.gnu.org/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=bkhdx6fcofvnwlvihx2n3zfq5denl756fdwzihlvgjb6ebnwyi@27jgemfrz7oo \
--to=steve@futurile.net \
--cc=cdo@mutix.org \
--cc=efraim@flashner.co.il \
--cc=guix-devel@gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/guix.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).