On Tue, Feb 21, 2023 at 10:27 AM Victoria Dye <vdye@xxxxxxxxxx> wrote: > > nit: period at the end of the sentence (s/lines/lines.) > > Also, this commit message explains *why* you're making the change, but not > what the commit actually does (that is, update the tests to adhere to the > new style). Would you mind adding a note about that to the message? For sure! Is it correct that I will need to amend the commit message and send out a v2 of the patch? > > > > -test_expect_success \ > > - 'check if contextually independent diffs for the same file apply' \ > > - '( git diff test~2 test~1 && git diff test~1 test~0 )| git apply' > > +test_expect_success 'check if contextually independent diffs for the same file apply' ' > > + ( git diff test~2 test~1 && git diff test~1 test~0 )| git apply > > +' > > As for this one, the test is correctly updated to the new style (per the > microproject prompt). However, the spacing around the '|' is a little weird > - I think there should be a space after ')'. On your next re-roll, could you > fix that spacing (in this patch is fine - it's not a substantial enough > change to warrant its own commit)? I'm not sure what you mean by "next re-roll," are you referring to v2? But then you said it was fine in this patch, so I'm confused. If I am going to be sending a v2, couldn't I just revert the last commit and add this change to the same commit?
