On Fri, Jan 03, 2025 at 03:26:45PM -0500, Jeff King wrote:
> On Fri, Jan 03, 2025 at 01:05:48PM +0100, Patrick Steinhardt wrote:
>
> > > diff --git a/t/test-lib.sh b/t/test-lib.sh
> > > index c9487d0805..d1f62adbf8 100644
> > > --- a/t/test-lib.sh
> > > +++ b/t/test-lib.sh
> > > @@ -1177,7 +1177,8 @@ check_test_results_san_file_empty_ () {
> > > ! find "$TEST_RESULTS_SAN_DIR" \
> > > -type f \
> > > -name "$TEST_RESULTS_SAN_FILE_PFX.*" 2>/dev/null |
> > > - xargs grep -q ^DEDUP_TOKEN
> > > + xargs grep ^DEDUP_TOKEN |
> > > + grep -qv sanitizer::GetThreadStackTopAndBottom
> > > }
> >
> > It would be nice to provide some more context here in the form of a
> > comment so that one doesn't have to blame the commit.
>
> We can add that on top, but I'm not sure what it should say. Do you want
> something along the lines of "add false positives to ignore here..." or
> are an explanation of why we are ignoring this particular false
> positive?
The latter, ideally also with a reference to the upstream issue you have
created. That makes it way easier to discover why this line exists and
may prompt people to remove the line eventually if they discover that
the issue has been fixed for a while.
It doesn't have to be a full paragraph, but a sentence or to go a long
way sometimes. For more context people can still blame the commit
message.
Patrick
