Thread (13 messages) flat view 13 messages, 3 authors, 2017-06-21

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

From: Kaartic Sivaraam <hidden>
Date: 2017-06-20 13:37:40

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
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
-------
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 [off-list ref]
 Signed-off-by: Kaartic Sivaraam [off-list ref]
New one
-------
 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 [off-list ref]
 Signed-off-by: Kaartic Sivaraam [off-list ref]
 Signed-off-by: Ævar Arnfjörð Bjarmason [off-list ref]
 
 * 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 [off-list ref]
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help