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

Re: [PATCH 1/3] Contextually notify user about an initial commit

From
Kaartic Sivaraam <kaarticsivaraam91196@gmail.com>
Date
Jun 20, 2017, 13:37 UTC
Message-ID
<1497965839.2792.2.camel@gmail.com>
In-Reply-To
<87o9tj86hs.fsf@gmail.com>
Thanks for trying to help. A few comments regarding your suggestions.
>  * Let's do that spacing fix (unrelated fix) in its own commit.
> 
Ok. That's a good point.
>   * You should add tests along with the code being changed, and
>     especially change tests that would fail with your new code,
>     otherwise you break bisection.
> 

I'm not sure if the convention here is to keep tests along with change it tests. I thought it would be better to separate them as they are "separate" changes. They're separate because in case of any issues with the test it should be possible to identify them separately. This isn't possible when they are added along with the change. 

Further, adding them as a separate change change (commit) immediately after the change it tests would ease the task of finding the change it tests.

If the convention here, really, is to add tests along with the change, I can't do anything but to follow it. I guess it should be mentioned somewhere in the Documentation/SubmittingPatches. AFAIK, I don't think it's mentioned there.

>  * I think the commit message here could be shorter & clearer at the
>    same time.

I don't think it's unclear. It's longer for the reason that follows. The commit message was crafted based on the following guideline found in Documentation/SubmittingPatches

Show 10 quoted lines
> The body should provide a meaningful commit message, which:
> 
>   . explains the problem the change tries to solve, i.e. what is 
>     wrong with the current code without the change.
> 
>   . justifies the way the change solves the problem, i.e. why the
>     result with the change is better.
> 
>   . alternate solutions considered but discarded, if any.
> 

I don't think the new commit message follows this. Both the commit messages are found below for comparison.

Old one -------

Show 29 quoted lines
> On Tue, Jun 20 2017, Kaartic Sivaraam jotted:
> 
>  "git status" indicated "Initial commit" when HEAD points at
>  an unborn branch.  This message is shared with the commit
>  log template "git commit" prepares for the user when
>  creating a commit (i.e. "You are about to create the initial
>  commit"), and is OK as long as the reader is aware of the
>  nature of the message (i.e. it guides the user working
>  toward the next commit), but was confusing to new users,
>  especially the ones who do "git commit -m message" without
>  having a chance to pay attention to the commit log template.
>  
>  The "Initial commit" indication wasn't an issue in the commit
>  template. Taking that into consideration, a good solution would
>  be to contextually use different messages to indicate the user
>  that there were no commits in this branch.
> 
>  A few alternatives considered were,
>  
>  * Waiting for initial commit
>  * Your current branch does not have any commits
>  * Current branch waiting for initial commit
>  
>  The most succint one, "No commits yet", among the alternatives
>  was chosen.
>  
>  Helped-by: Junio C Hamano <gitster@pobox.com>
>  Signed-off-by: Kaartic Sivaraam <kaarticsivaraam91196@gmail.com>
> 

New one -------

Show 16 quoted lines
>  status: contextually notify user about an initial commit
> 
>  Change the output of "status" to say "No commits yet" when "git
>  retaining the current "Initial commit" message displayed in the
>  template that's displayed in the editor when the initial commit is
>  being authored.
> 
>  The existing "Initial commit" message makes sense for the commit
>  template where we're making the initial commit, but is confusing 
>  when merely checking the status of a fresh repository without 
>  having any commits yet.
> 
>  Helped-by: Junio C Hamano <gitster@pobox.com>
>  Signed-off-by: Kaartic Sivaraam <kaarticsivaraam91196@gmail.com>
>  Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
>  
>  * The commit message doesn't follow our usual format.
Could you be more specific about where it's not following the format?
-- 
Regards,
Kaartic Sivaraam <kaarticsivaraam91196@gmail.com>
Previous: Ævar Arnfjörð BjarmasonNext: Ævar Arnfjörð Bjarmason
Message 37 of 48 in “wt-status.c: Modified status message shown for a parent-less branch”
  1. wt-status.c: Modified status message shown for a parent-less branchKaartic Sivaraam, Jun 10, 2017
  2. Kaartic SivaraamJun 10, 2017
  3. Junio C HamanoJun 10, 2017
  4. Kaartic SivaraamJun 10, 2017
  5. Kaartic SivaraamJun 10, 2017
  6. Jeff KingJun 10, 2017
  7. Junio C HamanoJun 10, 2017
  8. Kaartic SivaraamJun 12, 2017
  9. Junio C HamanoJun 12, 2017
  10. Jeff KingJun 12, 2017
  11. Junio C HamanoJun 12, 2017
  12. Jeff KingJun 12, 2017
  13. Kaartic SivaraamJun 15, 2017
  14. Jeff KingJun 15, 2017
  15. Samuel LijinJun 15, 2017
  16. Jeff KingJun 15, 2017
  17. Kaartic SivaraamJun 16, 2017
  18. Jeff KingJun 16, 2017
  19. Kaartic SivaraamJun 18, 2017
  20. Contextually notify user about an initial commitKaartic Sivaraam, Jun 18, 2017
  21. Ævar Arnfjörð BjarmasonJun 18, 2017
  22. 1/2 Contextually notify user about an initial commitKaartic Sivaraam, Jun 19, 2017
  23. 2/2 Add test for the new status messageKaartic Sivaraam, Jun 19, 2017
  24. Junio C HamanoJun 19, 2017
  25. Kaartic SivaraamJun 19, 2017
  26. Jeff KingJun 19, 2017
  27. Kaartic SivaraamJun 19, 2017
  28. Junio C HamanoJun 19, 2017
  29. 2/2 Add test for the new status messageKaartic Sivaraam, Jun 19, 2017
  30. Jeff KingJun 19, 2017
  31. Kaartic SivaraamJun 19, 2017
  32. Junio C HamanoJun 19, 2017
  33. 1/3 Contextually notify user about an initial commitKaartic Sivaraam, Jun 20, 2017
  34. 2/3 Update test(s) that used old status messageKaartic Sivaraam, Jun 20, 2017
  35. 3/3 Add tests for the contextual initial status messageKaartic Sivaraam, Jun 20, 2017
  36. Ævar Arnfjörð BjarmasonJun 20, 2017
  37. Kaartic SivaraamJun 20, 2017
  38. Ævar Arnfjörð BjarmasonJun 20, 2017
  39. Kaartic SivaraamJun 21, 2017
  40. status: contextually notify user about an initial commitKaartic Sivaraam, Jun 21, 2017
  41. Kaartic SivaraamJun 21, 2017
  42. Ævar Arnfjörð BjarmasonJun 21, 2017
  43. Kaartic SivaraamJun 21, 2017
  44. Junio C HamanoJun 21, 2017
  45. status: contextually notify user about an initial commitKaartic Sivaraam, Jun 21, 2017
  46. Junio C HamanoJun 22, 2017
  47. Kaartic SivaraamJun 22, 2017
  48. Philip OakleyJun 10, 2017

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.