Re: [PATCH v4 17/29] tests: t3440: create expect files at point of use

Hi Phillip,

 ---- On Thu, 23 Oct 2025 17:04:34 +0800  Phillip Wood <phillip.wood123@gmail.com> wrote --- 
 > On 15/10/2025 14:58, Li Chen wrote:
 > > Hi Kristoffer,
 > > 
 > > Thanks for the review suggestions! I'll address them in the next version.
 > > 
 > >   ---- On Wed, 15 Oct 2025 04:41:33 +0800  Kristoffer Haugsbakk <kristofferhaugsbakk@fastmail.com> wrote ---
 > >   > Now you start to change the test suite/file that you created for this
 > >   > series.  There shouldn’t be a need to do a test file-only patch/commit
 > >   > for a fresh series.
 > >   >
 > >   > I saw in one of your patches that you removed `--keep-empty` from a test
 > >   > because “that is the default”.  I also saw Phillip’s comment somewhere
 > >   > that said the same thing.
 > >   >
 > >   > The goal with maturing series is not to add patches on top in each round
 > >   > (if that’s what you are doing).  It is to recreate them as if the series
 > >   > was perfectly written to begin with; if one patch introduces
 > >   > `--trailers` and a test file, then there should be no need with
 > >   > follow-up patches that improve the test file style, refactors it, and
 > >   > so on.
 > > 
 > > Thanks for the tip. I split the changes into separate commits to ease review,
 > >   as Phillip suggested in https://lore.kernel.org/git/d4c9f082-52be-48d9-b817-fcb8a72e1bd7@gmail.com/.
 > > 
 > > It seems I may have overdone it? If so, I'll try for a better balance in the next version.
 > I asked that you did not refactor code at the same time as you moved it. 
 > I was expecting a handful of patches, not twenty-nine. The point that 
 > Kristoffer makes about this patch is perfectly valid - you add a new 
 > test and then correct it in a later patch. Instead you should correct 
 > the test where it is introduced as Kirstoffer suggested. Looking at the 
 > first patch in this series there seems to have been some 
 > miscommunication because it has exactly the same problem as V3. The code 
 > that is moved from builtin/interpret-trailers.c to trailer.c is heavily 
 > refactored at the same time. Variable names are changed and the code is 
 > rearranged so that "git diff --color-moved 
 > --color-moved-ws=ignore-indentation-change" detects barely any moved 
 > lines. I'll try and leave some more detailed feedback on the first few 
 > patches of V5 in the next few days.

I mistakenly misunderstood that you meant changes between each patchset version should be reflected by adding new patches. 
Now I understand that you mean refactoring the original code needs to be reflected in new patches for review. Thank you very 
Thank you for telling me about --color-moved, and I found that git log also has this parameter. This option is very amazing.
 I will do as you requested in the next version. 

I sincerely apologize for the misunderstanding and wasted time.

Regards, Li​