From: Junio C Hamano <gitster@pobox.com>
To: Eric Sunshine <sunshine@sunshineco.com>
Cc: Patrick Steinhardt <ps@pks.im>, git@vger.kernel.org
Subject: Re: [PATCH] tests: prefer host Git to verify chainlint self-checks
Date: Thu, 14 Dec 2023 08:16:45 -0800 [thread overview]
Message-ID: <xmqqbkaspxn6.fsf@gitster.g> (raw)
In-Reply-To: <CAPig+cQ2-PB24n0xfcoSy_1UT-VbEZUXXJ9QbA8FBA8Vfyd6Ng@mail.gmail.com> (Eric Sunshine's message of "Thu, 14 Dec 2023 03:39:17 -0500")
Eric Sunshine <sunshine@sunshineco.com> writes:
>> - Fix the ".expect" files so that we can avoid all of these games.
>>
>> I actually like the last option most. I'll have a go at it and send this
>> third version out in a bit.
>
> I sent a reply[1] in the other thread explaining why I'm still leaning
> toward `sed` to smooth over these minor differences rather than
> churning the "expect" files, especially since the minor differences
> are not significant to what is actually being tested.
If it is just one time bulk conversion under t/chainlint/ to match
what the chainlint.pl script produces, with the possibility of
similar bulk updates in the future when the script gets updated, I
tend to agree with Patrick that getting rid of the fuzzy comparison
will be the best way forward.
Especially if the fuzzy comparison is done only to hide differences
between what the old chainlint.sed used to produce and what the
current version produces, that is. If for some reason the script
started to create subtly different output for other reasons (e.g.,
it may produce different whitespaces on a particular platform, or
with a specific version of perl interpreter), we'd better be aware
of it, instead of blindly ignoring the differences without
inspecting them and verifying that they are benign.
By going through the single conversion pain, it will force us to
think twice before breaking its output stability while updating
chainlint.pl, which would also be a good thing.
THanks.
next prev parent reply other threads:[~2023-12-14 16:16 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-12-12 11:32 [PATCH] tests: prefer host Git to verify chainlint self-checks Patrick Steinhardt
2023-12-12 19:30 ` Junio C Hamano
2023-12-13 7:20 ` Patrick Steinhardt
2023-12-13 15:11 ` Junio C Hamano
2023-12-14 3:33 ` Eric Sunshine
2023-12-14 8:13 ` Patrick Steinhardt
2023-12-14 8:39 ` Eric Sunshine
2023-12-14 8:40 ` Patrick Steinhardt
2023-12-14 16:16 ` Junio C Hamano [this message]
2023-12-14 18:10 ` Eric Sunshine
2023-12-14 19:13 ` Junio C Hamano
2023-12-15 5:33 ` Patrick Steinhardt
2023-12-14 8:30 ` [PATCH v2] tests: adjust whitespace in chainlint expectations Patrick Steinhardt
2023-12-14 8:44 ` Eric Sunshine
2023-12-15 6:04 ` [PATCH v3] " Patrick Steinhardt
2023-12-15 6:24 ` Eric Sunshine
2023-12-15 6:29 ` Patrick Steinhardt
2023-12-15 6:40 ` Eric Sunshine
2023-12-15 6:42 ` [PATCH v4] " Patrick Steinhardt
2023-12-15 7:17 ` Eric Sunshine
2023-12-15 16:44 ` 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=xmqqbkaspxn6.fsf@gitster.g \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=ps@pks.im \
--cc=sunshine@sunshineco.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 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 NNTP newsgroup(s).