[2/3] t3206: demonstrate failure with notes
diff mbox series

Message ID 00d6b47db0a68d0bc91b252675a2165985426f5e.1574125554.git.liu.denton@gmail.com
State New
Headers show
Series
  • range-diff: don't compare notes
Related show

Commit Message

Denton Liu Nov. 19, 2019, 1:06 a.m. UTC
When a commit being range-diff'd has a note attached to it, the note
will be compared as well. However, this shouldn't happen since the
purpose of range-diff is to compare the difference between two
commit-ranges and, since the note attached to a commit is mutable, they
shouldn't be included as part of the comparison.

Demonstrate this failure in t3206.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 t/t3206-range-diff.sh | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

Comments

Junio C Hamano Nov. 19, 2019, 3:03 a.m. UTC | #1
Denton Liu <liu.denton@gmail.com> writes:

> When a commit being range-diff'd has a note attached to it, the note
> will be compared as well. However, this shouldn't happen since the
> purpose of range-diff is to compare the difference between two
> commit-ranges and, since the note attached to a commit is mutable, they
> shouldn't be included as part of the comparison.

I do not agree with the reasoning.  Commits in the new iteration of
the same series would have note attached to them that are different
from the note attached to corresponding commits in the old iteration.

Imagine that "git am" gets enhanced to store the per-iteration
comments you write under the three-dash lines (e.g. "fixed typo in
the log message pointed out by X") as notes attached to each of the
resulting commits.  It does make sense to compare the commits _with_
notes by default, if these notes are configured to be shown in "git
show" output by default for the user.

I do think that it may be a good idea to optionally allow comparison
without notes, when "--no-notes" is given to range-diff on the
command line, by passing the option through, though.

Thanks.

Patch
diff mbox series

diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh
index 87c6c029db..29b9a6f854 100755
--- a/t/t3206-range-diff.sh
+++ b/t/t3206-range-diff.sh
@@ -505,4 +505,19 @@  test_expect_success 'range-diff overrides diff.noprefix internally' '
 	git -c diff.noprefix=true range-diff HEAD^...
 '
 
+test_expect_failure 'range-diff does not compare notes' '
+	git notes add -m "topic note" topic &&
+	git notes add -m "unmodified note" unmodified &&
+	test_when_finished git notes remove topic unmodified &&
+	git range-diff --no-color master..topic master..unmodified \
+		>actual &&
+	cat >expected <<-EOF &&
+	1:  $(test_oid t1) = 1:  $(test_oid u1) s/5/A/
+	2:  $(test_oid t2) = 2:  $(test_oid u2) s/4/A/
+	3:  $(test_oid t3) = 3:  $(test_oid u3) s/11/B/
+	4:  $(test_oid t4) = 4:  $(test_oid u4) s/12/B/
+	EOF
+	test_cmp expected actual
+'
+
 test_done