Re: [eclipse7@gmx.net: [PATCH] diff: Only count lines in show_shortstats()]

On 06/07/2012 10:29 PM, Junio C Hamano wrote:

quoted

quoted

quoted

quoted

quoted

quoted

--- a/t/t4012-diff-binary.sh
+++ b/t/t4012-diff-binary.sh

@@ -36,6 +36,14 @@ test_expect_success '"apply --stat" output for binary file change' '
 	test_i18ncmp expected current
 '
 
+cat > expected <<\EOF
+ 4 files changed, 2 insertions(+), 2 deletions(-)
+EOF
+test_expect_success 'diff with --shortstat' '
+	git diff --shortstat >current &&
+	test_cmp expected current
+'
+

The test is OK, and follows the style of surrounding tests, but current
style is slightly different:
- no space after '>'
- expected output is inlined if it is short
- test_i18ncmp is used, even if the message is not yet i18n-ized

Something like this:
test_expect_success 'diff --shortstat output for binary file change' '
	echo " 4 files changed, 2 insertions(+), 2 deletions(-)" >expect &&
	git diff --shortstat >current &&
	test_i18ncmp expect current
'

  Should I rewrite the test for this patch? Or should it be changed for the
whole file at once?

Please keep a bugfix patch to only fixes with tests.  Style fixes
should be done later after dust from more important changes (e.g. a
bugfix) settles.

Thanks.

Does this need a v2?

Zbyszek