Re: [PATCH v3 0/2] index-pack: fsck honor checks

[Patrick Steinhardt <ps@pks.im>]

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

On Fri, Jan 26, 2024 at 05:11:14PM -0500, John Cai wrote:
> Hi Junio,
> 
> On 26 Jan 2024, at 16:18, Junio C Hamano wrote:
> 
> > "John Cai via GitGitGadget" <gitgitgadget@gmail.com> writes:
> >
> >>  1:  b3b3e8bd0bf ! 1:  cdf7fc7fe8a index-pack: test and document --strict=<msg>
> >>      @@ Metadata
> >>       Author: John Cai <johncai86@gmail.com>
> >>
> >>        ## Commit message ##
> >>      -    index-pack: test and document --strict=<msg>
> >>      +    index-pack: test and document --strict=<msg-id>=<severity>...
> >
> > Ah, I missed this one.  Nice spotting.
> >
> >>           5d477a334a (fsck (receive-pack): allow demoting errors to warnings,
> >>           2015-06-22) allowed a list of fsck msg to downgrade to be passed to
> >>      @@ Commit message
> >>           directly, (nor use index-pack for that matter) it is still useful to
> >>           document and test this feature.
> >>
> >>      +    Reviewed-by: Christian Couder <christian.couder@gmail.com>
> >>           Signed-off-by: John Cai <johncai86@gmail.com>
> >
> > I haven't seen Christian involved (by getting Cc'ed these patches,
> > sending out review comments, or giving his Reviewed-by:) during
> > these three rounds of this topic.  I'll wait until I hear from him
> > before queuing this, just to be safe.
> 
> Christian was involved on an off-list review of this patch series. You can see
> it in [1].
> 
> 1. https://gitlab.com/gitlab-org/git/-/merge_requests/88

I'm always a bit hesitant to add trailers referring to off-list reviews
to commits. It's impossible for a future reader to discover how that
trailer came to be by just using the mailing list archive, and expecting
them to use third-party services to verify them feels wrong to me.

It's part of the reason why I'm pushing more into the direction of
on-list reviews at GitLab. It makes it a lot more obvious how such a
Reviewed-by came to be and keeps things self-contained on the mailing
list. It also grows new contributors who are becoming more familiar with
how the Git mailing list works. If such a review already happened
internally due to whatever reason then I think it ought to be fine for
that reviewer to chime in saying that they have already reviewed the
patch series and that things look good to them.

Patrick

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