git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date
Apr 7, 2010, 14:28 UTC
Message-ID
<t2u51dd1af81004070728q491ae158gd6808f6ef4165b21@mail.gmail.com>
In-Reply-To
<7vsk77g7um.fsf@alter.siamese.dyndns.org>
On Wed, Apr 7, 2010 at 05:16, Junio C Hamano <gitster@pobox.com> wrote:
Show 51 quoted lines
> Æ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.

Previous: Junio C HamanoNext: Ævar Arnfjörð Bjarmason
Message 19 of 35 in “Does Git really need a commit message to go with a commit?”
  1. Ævar Arnfjörð BjarmasonApr 3, 2010
  2. Avery PennarunApr 4, 2010
  3. Add option to git-commit to allow empty log messagesÆvar Arnfjörð Bjarmason, Apr 4, 2010
  4. David AguilarApr 4, 2010
  5. Ævar Arnfjörð BjarmasonApr 4, 2010
  6. Junio C HamanoApr 5, 2010
  7. Sverre RabbelierApr 5, 2010
  8. Junio C HamanoApr 5, 2010
  9. Sverre RabbelierApr 5, 2010
  10. Miles BaderApr 5, 2010
  11. Junio C HamanoApr 5, 2010
  12. Jeff KingApr 5, 2010
  13. Ævar Arnfjörð BjarmasonApr 5, 2010
  14. Jonathan NiederApr 5, 2010
  15. Jonathan NiederApr 5, 2010
  16. Jeff KingApr 6, 2010
  17. Add option to git-commit to allow empty log messagesÆvar Arnfjörð Bjarmason, Apr 6, 2010
  18. Junio C HamanoApr 7, 2010
  19. Ævar Arnfjörð BjarmasonApr 7, 2010
  20. Ævar Arnfjörð BjarmasonApr 6, 2010
  21. Remove --allow-empty from the git-commit synopsisÆvar Arnfjörð Bjarmason, Apr 7, 2010
  22. Sverre RabbelierApr 7, 2010
  23. Remove --allow-empty from the git-commit synopsisÆvar Arnfjörð Bjarmason, Apr 7, 2010
  24. Sverre RabbelierApr 7, 2010
  25. Ævar Arnfjörð BjarmasonApr 7, 2010
  26. Tay Ray ChuanApr 7, 2010
  27. Remove --allow-empty from the git-commit synopsisÆvar Arnfjörð Bjarmason, Apr 7, 2010
  28. Johannes SixtApr 7, 2010
  29. Junio C HamanoApr 7, 2010
  30. Remove --allow-empty from the git-commit synopsisÆvar Arnfjörð Bjarmason, Apr 7, 2010
  31. Stephen BoydApr 7, 2010
  32. Junio C HamanoApr 7, 2010
  33. Jeff KingApr 8, 2010
  34. Jeff KingApr 6, 2010
  35. Santi BéjarApr 4, 2010

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.