Refusing to use PRs

Posted Jan 12, 2026 10:59 UTC (Mon) by alx.manpages (subscriber, #145117)
In reply to: Refusing to use PRs by mathstuf
Parent article: Evans: A data model for Git (and other docs updates)

> - PR changes things in "areas" A and B. I ask Alice to review A and Bob to review B. Using these, I can see that everything has been reviewed.

Usual practice in email is to reply to each patch with

Reviewed-by: My Name <me@example.com>

when you have finished reviewing the an entire patch.
With that you keep track of who reviewed each patch.

When the contributor sends a revision of the patch set, it will include those tags in the commits (if the reviewed commits haven't changed), which inform reviewers that certain patches have already been reviewed by some people.

When you've only reviewed part of a patch, you reply inline adding something more informal like 'LGTM' after the parts you've reviewed. Then maybe you reply to the parts you won't review by saying you won't review those, and ask someone else to review them.

I have never seen issues with that.

Some people are using b4(1) and/or Patchwork to automate some of that, but I still use bare git(1) and MUA for my work and I'm happy with its simplicity. I'm quite scared of the complexity of those tools (see what happened to Kees Cook recently with b4(1)).

> The other part is about managing open questions

I manage open questions by leaving email marked as unread. So far it works reasonably well.

to post comments

Refusing to use PRs

Posted Jan 12, 2026 20:01 UTC (Mon) by mathstuf (subscriber, #69389) [Link] (2 responses)

> Usual practice in email is to reply to each patch with

Partial patch reviews are still missing. Also doesn't handle split patches and the like.

> With that you keep track of who reviewed each patch.

The problem is that not everyone has the entire list archive on hand at all times. If I hop onto a patch series in situ, I may be missing discussion history because it's not well-linked on lists. Personally, I prefer to reply to series 1's 00 patch with further series, but some maintainers don't like that because they toss the whole thread around. But that sounds like one can "spoil" a thread by replying with a (proposed) patch of their own which seems…unfortunate.

> When the contributor sends a revision of the patch set, it will include those tags in the commits (if the reviewed commits haven't changed), which inform reviewers that certain patches have already been reviewed by some people.

Will it? `b4` probably does it, but I've always ended up copying things manually. There's also no verification that the tags are actually truly sourced.

> When you've only reviewed part of a patch, you reply inline adding something more informal like 'LGTM' after the parts you've reviewed. Then maybe you reply to the parts you won't review by saying you won't review those, and ask someone else to review them.

I have no idea how I would keep track of this team-wide in any efficient manner without structured data. Trailers probably don't suffice (Partially-reviewed-by? what parts?).

> I manage open questions by leaving email marked as unread. So far it works reasonably well.

The thing is that such state is local to you. I work with teams of developers and knowing Sally has an open question about something can help prevent me from prematurely merging it because I cannot see her unread state (or even interpret it if I do see it).

Refusing to use PRs

Posted Jan 12, 2026 22:46 UTC (Mon) by alx.manpages (subscriber, #145117) [Link] (1 responses)

> > With that you keep track of who reviewed each patch.
>
> The problem is that not everyone has the entire list archive on hand at all times. If I hop onto a patch series in situ, I may be missing discussion history because it's not well-linked on lists. Personally, I prefer to reply to series 1's 00 patch with further series, but some maintainers don't like that because they toss the whole thread around. But that sounds like one can "spoil" a thread by replying with a (proposed) patch of their own which seems…unfortunate.

I also prefer like you having all v2,v3,... be replies to v1's 00 patch. As you said, that makes all the context available to a reader without much effort.

I just realized I haven't documented this in the contributing guidelines. I should document this now.

> > When the contributor sends a revision of the patch set, it will include those tags in the commits (if the reviewed commits haven't changed), which inform reviewers that certain patches have already been reviewed by some people.
>
> Will it? `b4` probably does it, but I've always ended up copying things manually. There's also no verification that the tags are actually truly sourced.

I do it manually. (Very) recently, I started appending a 'Message-ID:' tag right below each review tag, documenting the mail from which I took the tag. That makes sure I don't make mistakes, or at least that if I make mistakes one can easily check them. I don't expect frequent mistakes.

Another case is malicious tags. If one adds false tags, someone would likely realize and then bust the author. FWIW, I've seen invalid tags added in a patch generated by an LLM. We completely prohibit LLMs in the Linux man-pages project, and this served as yet another proof that the contribution was from an LLM, so I banned the user.

Refusing to use PRs

Posted Jan 12, 2026 23:45 UTC (Mon) by alx.manpages (subscriber, #145117) [Link]

Regarding replying to previous iterations of a patch set, I've documented it now:

<https://git.kernel.org/pub/scm/docs/man-pages/man-pages.g...>