Re: [PATCH v2] Add option to git-commit to allow empty log messages

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Wed, Apr 7, 2010 at 05:16, Junio C Hamano <gitster@pobox.com> wrote:
> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>
>> diff --git a/t/t7510-commit-allow-empty-message.sh b/t/t7510-commit-allow-empty-message.sh
>> new file mode 100755
>> index 0000000..d7dc0da
>
> I do not think a separate script is worth it.  I'd just add tests to t7500.
>
>> +test_expect_success 'a basic commit in an empty tree should succeed' '
>> +     (
>> +             echo content > foo &&
>> +             git add foo &&
>> +             git commit -m "initial commit"
>> +     ) &&
>> +     commit_msg_is "initial commit"
>> +'
>
> No need for this, especially if this becomes part of t7500.
>
>> +test_expect_success 'Commit no message with --allow-empty-message' '
>> +     (
>> +             echo "more content" >> foo &&
>> +             git add foo &&
>> +             printf "" | git commit --allow-empty-message
>> +     ) &&
>> +     commit_msg_is ""
>> +'
>
> No need for subprocess, nor printf piped to the command.
>
>> +test_expect_success 'Commit a message with --allow-empty-message' '
>> +     (
>> +             echo "even more content" >> foo &&
>> +             git add foo &&
>> +             git commit --allow-empty-message -m"hello there"
>> +     ) &&
>> +     commit_msg_is "hello there"
>> +'
>
> Ditto.
>
> You are falling into the same trap as everybody else does when showing off
> your shiny new toy.  A missing but very necessary test is that "commit"
> with your patch does still fail when an empty message is given without the
> new option.
>
> It takes a conscious effort to carefully make sure that your shiny new toy
> does not trigger when it shouldn't, especially when you are so excited by
> seeing it work when it should.
>
> But that is part of the art of writing good test scripts.

Thanks for shepherding the patch and committing it, and for pointing
out what I could have done better. I'll try not to make similar
mistakes with future submissions.