git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "René Scharfe" <l.s.r@web.de>
To: Felipe Contreras <felipe.contreras@gmail.com>
Cc: Junio C Hamano <gitster@pobox.com>, git@vger.kernel.org
Subject: Re: [PATCH] branch: use $curr_branch_short more
Date: Sun, 15 Sep 2013 13:42:43 +0200	[thread overview]
Message-ID: <52359D33.508@web.de> (raw)
In-Reply-To: <CAMP44s0LOhGWpgy6iy==WC7tBtjUjG-CTJ6jXNm+Jumu-5OkXw@mail.gmail.com>

Am 09.09.2013 01:13, schrieb Felipe Contreras:
> On Sun, Sep 8, 2013 at 10:21 AM, René Scharfe <l.s.r@web.de> wrote:
>> Am 31.08.2013 19:20, schrieb Felipe Contreras:
>>
>>> A summary should contain as much information that would allow me to
>>> skip the commit message as possible.
>>>
>>> If I can't tell from the summary if I can safely skip the commit
>>> message, the summary is not doing a good job.
>>>
>>> "trivial simplification" explains the "what", and the "why" at the
>>> same time, and allows most people to skip the commit message, thus is
>>> a good summary.
>>
>>
>> No patch should be skipped on the mailing list.  As you wrote, trivial
>> patches can still be wrong.
>
> What patches each persons skips is each person's own decision, don't
> be patronizing, if I want to skip a trivial patch, I will, I can't
> read each and every patch from the dozens of mailing lists I'm
> subscribed to, and there's no way each and every reader is going to
> read each and every patch. They should be prioritized, and trivial
> ones can be safely skipped by most people.

Yes, of course; someone needs to review every patch in the end, but each 
reader decides for themselves which ones to skip.  I can't keep up with 
the traffic either.

By the way, the bikeshedding phenomenon probably causes trivial patches 
to receive the most attention. :)

>> When going through the history I can see that quickly recognizing
>> insubstantial changes is useful, but if I see a summary twice then in my
>> mind forms a big question mark -- why did the same thing had to be done yet
>> again?
>>
>> As an example, both 0d12e59f (pull: replace unnecessary sed invocation) and
>> bc2bbc45 (pull, rebase: simplify to use die()) could arguably have had the
>> summary "trivial simplification", but I'm glad the author went with
>> something a bit more specific.
>
> Well I wont. Because it takes long to read, and after reading I still
> don't don't if they are trivial or not, I might actually have to read
> the commit message, but to be honest, I would probably go right into
> the diff itself, because judging from Git's history, chances are that
> somebody wrote a novel there with the important bit I'm looking for
> just at the end, to don't ruin the suspense.

Ha!  It's better to write it down at all than to miss it years later, 
when even the original author has forgotten all about it.

> In the first commit, it's saying it's a single invocation, so I take
> it it's trivial, but what is it replaced with? Is the code simpler, is
> it more complex? I don't know, I'm still not being told *why* that
> patch is made. It says 'unnecessary' but why is it unnecessary?

The sed call is unnecessary because of the fact that it can be replaced. 
:)  I'm sure I would have understood it to mean a conversion to Shell 
builtin functionality in order to avoid forking and executing an 
external command, even if I hadn't read the patch.

> In the second commit, it's saying it's a simplification, but I don't
> know if it's just one instance, or a thousand, so I don't know what
> would be the impact of the patch.
>
> Either way I'm forced to read more just to know if it was safe for me
> to skip them, at which point the whole purpose of a summary is
> defeated.

I don't see how using "trivial simplification" as the summary for both 
could have improved matters.

>>> Again, triviality and correctness are two separate different things.
>>> The patch is trivial even if you can't judge it's correctness.
>>
>> Well, in terms of impact I agree.
>
> No, in all terms. A patch can be:
>
> Trivial and correct
> Trivial and incorrect
> Non-trivial and correct
> Non-trivial and incorrect

Well, yes, but I thought your statement that "The patch is trivial" was 
referring to my actual patch which started this sub-thread.  And I meant 
that the benefit of that patch to users and programmers was small.

>>> To me, what you are describing is an obvious patch, not a trivial one.
>>> An obvious patch is so obvious that you can judge it's correctness
>>> easily by looking at the diff, a trivial one is of little importance.
>>
>> That's one definition; I think I had the mathematical notion in mind that
>> calls proofs trivial which are immediately evident.
>
> We are not talking about mathematics, we are talking about the English
> human language.

Here we were talking about source code patches.  As formal descriptions 
of changes to (mostly) programming language text they are closer to 
mathematics than English.  Using math terms when talking about them is 
not too far of a stretch.

René

  reply	other threads:[~2013-09-15 11:43 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-08-30 21:56 [PATCH 0/6] Trivial cleanups and fixes Felipe Contreras
2013-08-30 21:56 ` [PATCH 1/6] reset: trivial refactoring Felipe Contreras
2013-08-31  3:36   ` Junio C Hamano
2013-08-30 21:56 ` [PATCH 2/6] branch: trivial style fix Felipe Contreras
2013-08-31  3:36   ` Junio C Hamano
2013-08-30 21:56 ` [PATCH 3/6] rebase: trivial style fixes Felipe Contreras
2013-08-31  3:49   ` Junio C Hamano
2013-08-30 21:56 ` [PATCH 4/6] reset: trivial style cleanup Felipe Contreras
2013-08-31  3:49   ` Junio C Hamano
2013-08-30 21:56 ` [PATCH 5/6] add: " Felipe Contreras
2013-08-31  3:37   ` Junio C Hamano
2013-08-30 21:56 ` [PATCH 6/6] pull: trivial cleanup Felipe Contreras
2013-08-31  3:58   ` Junio C Hamano
2013-08-31  7:56     ` Felipe Contreras
2013-08-31  8:10     ` [PATCH] branch: use $curr_branch_short more René Scharfe
2013-08-31  8:22       ` Felipe Contreras
2013-08-31  9:11         ` René Scharfe
2013-08-31  9:22           ` Felipe Contreras
2013-08-31 10:28             ` René Scharfe
2013-08-31 17:20               ` Felipe Contreras
2013-09-08 15:21                 ` René Scharfe
2013-09-08 23:13                   ` Felipe Contreras
2013-09-15 11:42                     ` René Scharfe [this message]
2013-09-15 13:02                       ` Felipe Contreras
2013-09-03 16:56         ` Junio C Hamano
2013-09-08 15:21       ` [PATCH] pull: " René Scharfe

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=52359D33.508@web.de \
    --to=l.s.r@web.de \
    --cc=felipe.contreras@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).