On 06/07/2012 10:29 PM, Junio C Hamano wrote: >>>> >> > --- 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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html