Re: [PATCH v4 2/2] tests(mingw): avoid very slow `mingw_test_cmp`

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

Hi Taylor,

On Fri, 18 Nov 2022, Taylor Blau wrote:

> On Fri, Nov 18, 2022 at 02:32:28PM +0100, Johannes Schindelin wrote:
> > > But I'm not sure the rewritten version does what you claim, at least in
> > > my own personal opinion.
> > >
> > > It is not helpful to say the original approach "saw a lot of backlash".
> >
> > It is the nicest thing I can say about it.
>
> I don't think you have to or should refer to the earlier round of review
> at all.

You misunderstand.

I am referring to the fact that Git for Windows has run with a very
different solution for this problem, for years, yet it was rejected upon
upstreaming, and had to be replaced by a completely different workaround.

It's not just a simple "earlier round of review" at all that is the issue
I am describing.

It is a very real concern of future readers who know what patches are
currently in Git for Windows and who all of a sudden do not find the `git
test-tool cmp` code anymore in Git for Windows and then see that `git diff
--no-index` is used and naturally want to know what the heck happened.

This is context relevant to understand why the particular approach
implemented in the patch was chosen and another one was discarded (when
that other approach has served Git for Windows very well for several
years), for which the commit message is precisely the appropriate place. I
am quite lost trying to understand why I am asked to remove said context,
leaving future readers puzzled e.g. in the case that it should turn out to
have been a terrible idea to use the quite complex diff machinery for as
simple a task as `test_cmp`. It sounds to me like I am asked to make my
contribution worse ("worsimprove" is the term I recently learned to
describe this) instead of helping me to improve it.

The advice you provided directly contradicts what is written in
https://git-scm.com/docs/SubmittingPatches#describe-changes, after all
(ignore the funny grammar for now unless you want to add a tangent to this
already long thread):

	The body should provide a meaningful commit message, which:

	    [...]

	    3. alternate solutions considered but discarded, if any.

Thanks,
Johannes