On Thu, 25 Jul 2024 17:52:25 +0200, Kristoffer Haugsbakk wrote:
> Hi
>
> On Thu, Jul 25, 2024, at 16:41, David Disseldorp wrote:
> > 90bc19b3ae ("notes.c: introduce '--separator=<paragraph-break>' option")
> > changed note_data.given logic such that it's no longer set if a zero
> > length file or blob object is provided.
>
> This project uses the `git show -s --pretty=reference` format:
>
> 90bc19b3ae (notes.c: introduce '--separator=<paragraph-break>'
> option, 2023-05-27)
Okay, will fix in a subsequent version.
> > t/t3301-notes.sh | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh
> > index 536bd11ff4..c0dbacc161 100755
> > --- a/t/t3301-notes.sh
> > +++ b/t/t3301-notes.sh
> > @@ -1557,4 +1557,9 @@ test_expect_success 'empty notes are displayed by
> > git log' '
> > test_cmp expect actual
> > '
> >
> > +test_expect_success 'empty notes do not invoke the editor' '
> > + test_commit 18th &&
> > + GIT_EDITOR="false" git notes add -C "$empty_blob" --allow-empty
> > +'
> > +
> > test_done
> > --
> > 2.43.0
>
> This test fails, obviously. Maybe you can reorder the patches so that
> both two patches pass the test suite?
>
> Introducing a regression test in one patch and then fixing the bug (and
> making the test pass) in the next patch is a style that some prefer. But
> I have received feedback before that it’s best to avoid that.
Makes sense, thanks for the feedback.
