From: Junio C Hamano <gitster@pobox.com>
To: Kyle Lippincott <spectral@google.com>
Cc: Kyle Lippincott via GitGitGadget <gitgitgadget@gmail.com>,
git@vger.kernel.org, Patrick Steinhardt <ps@pks.im>
Subject: Re: [PATCH v2 3/3] t6421: fix test to work when repo dir contains d0
Date: Fri, 02 Aug 2024 17:27:32 -0700 [thread overview]
Message-ID: <xmqqcymq8n7v.fsf@gitster.g> (raw)
In-Reply-To: <CAO_smVh-gQWy7xUJgFjd6gUWCTV5jTYJ9E9E3rvaQg0EY-2BdQ@mail.gmail.com> (Kyle Lippincott's message of "Fri, 2 Aug 2024 17:03:45 -0700")
Kyle Lippincott <spectral@google.com> writes:
>> So it really didn't have to clarify what it is looking for, as it
>> would not help seeing what false positives the patterns are designed
>> to avoid matching. Sorry about that.
>
> I would have included examples, but they're quite long (>>>80 chars),
> so seemed very out of place in both commit description and in the
> codebase.
Absolutely. It turned out not to be so useful to show the shape of
potential matches, like this one:
... run-command.c:733 | d0 | main | child_start |...
To explain why spaces around " d0 " matters, the readers need to
understand that other trace entries that are irrelevant for our
purpose, like this one
... | label:do_write_index /path/to/t/trash directory.../.git/index.lock
we want reject, and for that we want the pattern to be specific
enough by looking for " do " that is followed by " child_start ".
Otherwise the leading paths that can contain anything won't easily
match, and the original of looking for just "d0" was way too error
prone. But it is hard to leave a concise hint for that there.
So, again, sorry about the bad suggestion.
> I considered using `awk -F"|" "\$2~/d0/ &&
> \$9~/fetch\\.negotiationAlgorithm/{ print }" trace.output >fetches`,
> but it was longer, possibly less clear, and less specific (since it
> didn't include the $4~/child_start/ condition)
Yeah, using the syntactic clue -F"|" would also be a way to convey
the intention (i.e. "we are dealing with tabular output and we
expect nth column to be X"), but what you have is probably good
enough---it certainly is simpler to read and understand. I briefly
considered that looking for "| d0 |" (i.e. explicitly mentioning the
column separator in the pattern) would make it even more obvious
what we are looking for, but having to worry about quoting "|" in
regexp would negate the benefit of obviousness out of the approach
to use "grep".
Thanks.
next prev parent reply other threads:[~2024-08-03 0:27 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-08-02 4:10 [PATCH 0/3] Small fixes for issues detected during internal CI runs Kyle Lippincott via GitGitGadget
2024-08-02 4:10 ` [PATCH 1/3] set errno=0 before strtoX calls Kyle Lippincott via GitGitGadget
2024-08-02 5:12 ` Patrick Steinhardt
2024-08-02 6:15 ` Kyle Lippincott
2024-08-02 15:01 ` Junio C Hamano
2024-08-02 4:10 ` [PATCH 2/3] strbuf: set errno to 0 after strbuf_getcwd Kyle Lippincott via GitGitGadget
2024-08-02 15:10 ` Junio C Hamano
2024-08-02 17:56 ` Kyle Lippincott
2024-08-02 4:10 ` [PATCH 3/3] t6421: fix test to work when repo dir contains d0 Kyle Lippincott via GitGitGadget
2024-08-02 15:13 ` Junio C Hamano
2024-08-02 20:58 ` [PATCH v2 0/3] Small fixes for issues detected during internal CI runs Kyle Lippincott via GitGitGadget
2024-08-02 20:58 ` [PATCH v2 1/3] set errno=0 before strtoX calls Kyle Lippincott via GitGitGadget
2024-08-02 21:18 ` Junio C Hamano
2024-08-02 20:58 ` [PATCH v2 2/3] strbuf: set errno to 0 after strbuf_getcwd Kyle Lippincott via GitGitGadget
2024-08-02 21:32 ` Junio C Hamano
2024-08-02 21:54 ` Eric Sunshine
2024-08-05 15:51 ` Junio C Hamano
2024-08-06 6:26 ` Patrick Steinhardt
2024-08-06 7:04 ` Kyle Lippincott
2024-08-02 23:51 ` Kyle Lippincott
2024-08-05 17:12 ` Kyle Lippincott
2024-08-02 20:58 ` [PATCH v2 3/3] t6421: fix test to work when repo dir contains d0 Kyle Lippincott via GitGitGadget
2024-08-02 21:41 ` Junio C Hamano
2024-08-03 0:03 ` Kyle Lippincott
2024-08-03 0:27 ` Junio C Hamano [this message]
2024-08-05 17:10 ` [PATCH v3 0/2] Small fixes for issues detected during internal CI runs Kyle Lippincott via GitGitGadget
2024-08-05 17:10 ` [PATCH v3 1/2] set errno=0 before strtoX calls Kyle Lippincott via GitGitGadget
2024-08-05 17:10 ` [PATCH v3 2/2] t6421: fix test to work when repo dir contains d0 Kyle Lippincott via GitGitGadget
2024-08-05 18:37 ` [PATCH v3 0/2] Small fixes for issues detected during internal CI runs Junio C Hamano
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
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqcymq8n7v.fsf@gitster.g \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=gitgitgadget@gmail.com \
--cc=ps@pks.im \
--cc=spectral@google.com \
/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.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.