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

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Nov 20, 2022, 23:36 UTC
Message-ID: <q39q9rr7-or29-5510-5177-2s92n8qs2540@tzk.qr>
In-Reply-To: <Y3fLdJ7CIIU8fj0g@nand.local>

Hi Taylor,

On Fri, 18 Nov 2022, Taylor Blau wrote:

Show 10 quoted lines

> 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