Re: [PATCH] Add committer and author names to top of COMMIT_EDITMSG.

2 messages, 2 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH] Add committer and author names to top of COMMIT_EDITMSG.

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:44:04

"Stephen Sinclair" [off-list ref] writes:
quoted hunk
@@ -423,8 +423,18 @@ static int prepare_log_message(const char
*index_file, const char *prefix)
 			"#\n",
 			git_path("MERGE_HEAD"));

+    fprintf(fp, "\n");
+
+    fprintf(fp,
+            "# Committer: %s\n"
+            "# Author:    %s\n"
+            "#\n",
+            fmt_name(getenv("GIT_AUTHOR_NAME"),
+                     getenv("GIT_AUTHOR_EMAIL")),
+            fmt_name(getenv("GIT_COMMITTER_NAME"),
+                     getenv("GIT_COMMITTER_EMAIL")));
+
I'd almost agree with this patch if if added AUTHOR but not
COMMITTER, and only when AUTHOR is different from me.  That
would help reassure anybody while amending other's changes.
COMMITTER is always me and I should not reminded with extra
lines that waste precious screen real estate.

And no, I did not check if your change correctly supports the
use case of amending other's changes.  But if I recall the code
correctly, I suspect that your change doesn't.  The recorded
author is determined after the log message is prepared, way
later.

I strongly agree with Dscho that this change needs to be
defended with a good description on the reason why this is good.
If the reason is "newbie protection", I do not think this is a
good change at all.  Newbie protection is never a good reason to
make people who graduated that state to pay extra price
unconditionally.

Re: [PATCH] Add committer and author names to top of COMMIT_EDITMSG.

From: Stephen Sinclair <hidden>
Date: 2016-06-15 22:44:04

I'd almost agree with this patch if if added AUTHOR but not
COMMITTER, and only when AUTHOR is different from me.  That
would help reassure anybody while amending other's changes.
COMMITTER is always me and I should not reminded with extra
lines that waste precious screen real estate.
The purpose of my patch was to remind _myself_ what my name is, in
case I hadn't configured it correctly.
But I can see this use case also being useful.

And no, I did not check if your change correctly supports the
use case of amending other's changes.  But if I recall the code
correctly, I suspect that your change doesn't.  The recorded
author is determined after the log message is prepared, way
later.
Sure, that's possible.

I strongly agree with Dscho that this change needs to be
defended with a good description on the reason why this is good.
The patch was really to go along with my RFC about the idea.  I guess
it was too early to post a possible implementation.
(I have only just begun to look at the git code after all..)

If the reason is "newbie protection", I do not think this is a
good change at all.  Newbie protection is never a good reason to
make people who graduated that state to pay extra price
unconditionally.
I agree.  I wouldn't necessarily say it is "newbie protection", so
much as a friendly reminder of what username you are using while doing
a commit, which, as I said, might not be as expected if you have just
sat down at a new machine.  Especially if you have been using git for
a long time on a single machine, it is something you might easily
forget to configure.  (As I have, several times now.)  I agree,
however, that this it is not necessarily worth having this on the
screen every time you do a commit, for the exceptional instance where
it might be wrong.  Perhaps more usefully it could appear only if you
haven't yet created a user.email and user.name config entry.

Actually in my honest opinion, the default of using the computer's
host name and login is pretty much _never_ right, but I thought this
patch might be less intrusive than introducing a new error message.

I do have a slightly better patch now that has a more informative
message and uses git_committer_info() and git_author_info(), however
I'll wait for any more opinions before posting it.

In retrospect, I guess I could just as easily solve my problem by
introducing a post-receive hook for my personal repo that issues a
warning for commits not configured to my email address.


Steve
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help