Re: [PATCH v5] Add log.abbrevCommit config variable

Jay Soffian [off-list ref] writes:

+test_expect_success 'log.abbrev-commit configuration' '
+	test_when_finished "git config --unset log.abbrevCommit" &&
+
+	git log >expect.full &&
+	git log --abbrev-commit >expect.abbrev &&
+	git log --pretty=raw >expect.raw &&
+	git log -g --abbrev-commit --pretty=oneline >expect.reflog &&
+	git whatchanged --abbrev-commit >expect.whatchanged &&
+
+	git config log.abbrevCommit true &&
+
+	git log --no-abbrev-commit >actual.full &&
+	test_cmp expect.full actual.full &&
+
+	git log >actual.abbrev &&
+	test_cmp expect.abbrev actual.abbrev &&
+
+	git log --pretty=raw >actual.raw &&
+	test_cmp expect.raw actual.raw &&
+
+	git reflog >actual.reflog &&
+	test_cmp expect.reflog actual.reflog
+
+	git whatchanged >actual.whatchanged &&
+	test_cmp expect.whatchanged actual.whatchanged
+'

I can see you are testing all the commands in the "log" family for cases
where the new configuration _should_ take effect, but I cannot see the
same throughness for the negative case where the configuration should not
kick in. Is "log" the only one that you make sure --no-abbrev-commit
defeats the configuration? Please always test both sides of the coin.

Other than that, the patch looked fine from my cursory look.

Thanks.