All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kaartic Sivaraam <kaarticsivaraam91196@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: sbeller@google.com, git@vger.kernel.org
Subject: Re: [PATCH] hook: use correct logical variable
Date: Thu, 17 Aug 2017 08:17:38 +0530	[thread overview]
Message-ID: <1502938058.1710.14.camel@gmail.com> (raw)
In-Reply-To: <xmqqa830sptq.fsf@gitster.mtv.corp.google.com>

On Tue, 2017-08-15 at 10:28 -0700, Junio C Hamano wrote:
> I did shoot for conciseness, but what is a lot more important is to
> record what is at the core of the issue.  "I found it by doing A"
> can hint to careful readers why doing A leads to an undesirable
> behaviour, but when there are other ways to trigger problems that
> come from the same cause, "I found it by doing A" is less useful
> unless we also record "Doing A reveals the underlying problem X"
> that can be shared by other ways B, C, ... to trigger it.  The
> careful readers need to guess what the X is.
> 
> And once you identify the underlying problem X and record _that_ in
> the log message, I and A in "I found it by doing A" becomes much
> less interesting and the readers do not have to guess.
> 
> Your "A" is 'git commit --amend -s' with the disabled part of hook
> enabled.  But I think 'git commit' without "--amend" and "-s" would
> also show an issue that come from the same root cause.  The hook
> will add SoB that is based on the author, not the committer.  That
> resulting commit would be different from 'git commit -s' without the
> hook enabled, which would add SoB based on the commiter name (that
> would be a "B", that causes a related but different problem that
> comes from the same underlying issue "X" which is "we should
> consistently use the committer info like other parts of the
> system").
> 
> In any case, thanks for a fix-up.  Let's move this forward quickly,
> as it is an update to a topic that is already in 'master'.
> 
> Thanks.

Seeing the reply, I changed my opinion that "it isn't worth the effort
to write a better commit message for the change". I've given a try but
it got a little off-hand. Let me know if there's ways in which it could
be simplified and/or improved.

-- 
Kaartic

  reply	other threads:[~2017-08-17  2:46 UTC|newest]

Thread overview: 58+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-06-30 15:43 [PATCH] hooks: add signature to the top of the commit message Kaartic Sivaraam
2017-06-30 16:44 ` Junio C Hamano
2017-07-01 14:15   ` Kaartic Sivaraam
2017-07-01 16:16     ` "git intepret-trailers" vs. "sed script" to add the signature Kaartic Sivaraam
2017-07-01 17:32       ` [PATCH/RFC] hooks: add signature using "interpret-trailers" Kaartic Sivaraam
2017-07-03 16:58       ` "git intepret-trailers" vs. "sed script" to add the signature Junio C Hamano
2017-07-04 19:16         ` Kaartic Sivaraam
2017-07-05  1:48           ` Junio C Hamano
2017-07-05 17:00           ` [PATCH] hooks: add signature using "interpret-trailers" Kaartic Sivaraam
2017-07-05 17:35             ` Kaartic Sivaraam
2017-07-05 19:37               ` Junio C Hamano
2017-07-05 20:14               ` Ramsay Jones
2017-07-06 14:30                 ` Kaartic Sivaraam
2017-07-01 17:36     ` [PATCH] hooks: add signature to the top of the commit message Junio C Hamano
2017-07-01 18:40       ` Philip Oakley
2017-07-01 20:28         ` Junio C Hamano
2017-07-01 21:00           ` Philip Oakley
2017-07-01 18:52       ` Kaartic Sivaraam
2017-07-01 20:31         ` Junio C Hamano
2017-07-02 11:19           ` Kaartic Sivaraam
2017-07-02 11:27             ` [PATCH/RFC] hooks: replace irrelevant hook sample Kaartic Sivaraam
2017-07-05 16:51               ` [PATCH] " Kaartic Sivaraam
2017-07-05 19:50                 ` Junio C Hamano
2017-07-07 11:53                   ` Kaartic Sivaraam
2017-07-07 15:05                     ` Junio C Hamano
2017-07-07 15:24                       ` Kaartic Sivaraam
2017-07-07 16:07                         ` [PATCH 1/2] " Kaartic Sivaraam
2017-07-07 16:07                           ` [PATCH 2/2] hooks: add signature using "interpret-trailers" Kaartic Sivaraam
2017-07-07 18:27                           ` [PATCH 1/2] hooks: replace irrelevant hook sample Junio C Hamano
2017-07-10 14:17                             ` [PATCH 1/4] hook: cleanup script Kaartic Sivaraam
2017-07-10 14:17                               ` [PATCH 2/4] hook: name the positional variables Kaartic Sivaraam
2017-07-10 19:51                                 ` Junio C Hamano
2017-07-10 14:17                               ` [PATCH 3/4] hook: add signature using "interpret-trailers" Kaartic Sivaraam
2017-07-10 15:13                                 ` Ramsay Jones
2017-07-10 19:53                                   ` Junio C Hamano
2017-07-11 14:11                                     ` [PATCH 1/4] hook: cleanup script Kaartic Sivaraam
2017-07-11 14:11                                       ` [PATCH 2/4] hook: name the positional variables Kaartic Sivaraam
2017-07-11 14:11                                       ` [PATCH 3/4] hook: add sign-off using "interpret-trailers" Kaartic Sivaraam
2017-08-14  8:46                                         ` [PATCH] hook: use correct logical variable Kaartic Sivaraam
2017-08-14 17:54                                           ` Stefan Beller
2017-08-14 18:19                                           ` Junio C Hamano
2017-08-15  9:31                                             ` Kaartic Sivaraam
2017-08-15 17:28                                               ` Junio C Hamano
2017-08-17  2:47                                                 ` Kaartic Sivaraam [this message]
2017-08-17  2:50                                                   ` [PATCH v2/RFC] " Kaartic Sivaraam
2017-08-15  9:32                                             ` [PATCH] " Kaartic Sivaraam
2017-07-11 14:11                                       ` [PATCH 4/4] hook: add a simple first example Kaartic Sivaraam
2017-07-11 14:30                                         ` Kaartic Sivaraam
2017-07-11 13:10                                   ` [PATCH 3/4] hook: add signature using "interpret-trailers" Kaartic Sivaraam
2017-07-11 13:18                                   ` Kaartic Sivaraam
2017-07-10 14:17                               ` [PATCH 4/4] hook: add a simple first example Kaartic Sivaraam
2017-07-10 20:02                                 ` Junio C Hamano
2017-07-11 13:29                                   ` Kaartic Sivaraam
2017-07-11 16:03                                     ` Junio C Hamano
2017-07-11 18:04                                       ` Kaartic Sivaraam
2017-07-11 18:06                                       ` Kaartic Sivaraam
2017-07-10 19:50                               ` [PATCH 1/4] hook: cleanup script Junio C Hamano
2017-07-02 11:29             ` [PATCH] hooks: add script to HOOKS that allows adding notes from commit message Kaartic Sivaraam

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1502938058.1710.14.camel@gmail.com \
    --to=kaarticsivaraam91196@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=sbeller@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.