git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: John Keeping <john@keeping.me.uk>
To: Felipe Contreras <felipe.contreras@gmail.com>
Cc: "SZEDER Gábor" <szeder@ira.uka.de>,
	git@vger.kernel.org, "Junio C Hamano" <gitster@pobox.com>,
	"Ramkumar Ramachandra" <artagnon@gmail.com>,
	"Jonathan Nieder" <jrnieder@gmail.com>,
	"Martin von Zweigbergk" <martin.von.zweigbergk@gmail.com>
Subject: Re: [PATCH v4 10/45] sequencer: trivial fix
Date: Sun, 9 Jun 2013 20:01:03 +0100	[thread overview]
Message-ID: <20130609190103.GG22905@serenity.lan> (raw)
In-Reply-To: <CAMP44s1F5O3vjHuL=cGys=p_dbnnE-f-Mv+j5Q1ZbyVAJCjLSA@mail.gmail.com>

On Sun, Jun 09, 2013 at 12:53:38PM -0500, Felipe Contreras wrote:
> On Sun, Jun 9, 2013 at 12:37 PM, John Keeping <john@keeping.me.uk> wrote:
> > On Sun, Jun 09, 2013 at 07:33:42PM +0200, SZEDER Gábor wrote:
> >> On Sun, Jun 09, 2013 at 12:23:01PM -0500, Felipe Contreras wrote:
> >> > On Sun, Jun 9, 2013 at 12:18 PM, SZEDER Gábor <szeder@ira.uka.de> wrote:
> >> > > On Sun, Jun 09, 2013 at 11:40:22AM -0500, Felipe Contreras wrote:
> >> > >> We should free objects before leaving.
> >> > >>
> >> > >> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> >> > >
> >> > > A shortlog-friendlier subject could be: "sequencer: free objects
> >> > > before leaving".
> >> >
> >> > I already defended my rationale for this succinct commit message:
> >> >
> >> > http://thread.gmane.org/gmane.comp.version-control.git/225609/focus=225610
> >>
> >> Your arguments were unconvincing.  The mere fact that I raised this
> >> issue unbeknownst to the earlier posting clearly shows that there's
> >> demand for descriptive subjects.
> >
> > Not to mention that with your subject no body is needed, making the
> > overall message more succinct.
> 
> It's not succinct at all, because there's no short and quick
> description of what the patch actually is; a trivial fix.

Is it not equally succinct to say "fix memory leak"?

> > When reading a log, as soon as I see "trivial" I become suspicious that
> > someone is trying to cover something up, much like "left as an exercise
> > for the reader".  If the subject says "fix memory leak" then it's
> > obvious what the patch is meant to do, and when there is no subtlety to
> > be explained (as there isn't in this patch) there is no need for a body.
> 
> You are not a rational person then. The commit message has absolutely
> no bearing on the quality of the code. If you are less suspicious of a
> commit message that says "fix memory leak", you are being completely
> biased.
>
> Whether the commit message says "fix memory leak", or "trivial fix",
> or "foobar", the code might still be doing something wrong, and you
> can't decide that until you look at the code.

I have a certain level of trust that commit summaries in git.git will be
accurate.  If I want to know what has changed, then "fix memory leak"
implies "no functional change"; if I see "trivial fix" then how do I
know what that is?  It could be a whitespace change, a fix to a memory
leak, a typo correction, a change to a separator in a message shown to
the user, or even a small change to corner case behaviour.

> If you don't care about the code, but still want to know what the
> patch is doing, then you can look at the whole commit message, and "We
> should free objects before leaving." explains that perfectly.

The short message is what appears in "What's Cooking", why should I need
to break out of my mail client to find out what it means?

  reply	other threads:[~2013-06-09 19:01 UTC|newest]

Thread overview: 86+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-06-09 16:40 [PATCH v4 00/45] Massive improvents to rebase and cherry-pick Felipe Contreras
2013-06-09 16:40 ` [PATCH v4 01/45] build: generate and clean test scripts Felipe Contreras
2013-06-09 16:40 ` [PATCH v4 02/45] build: do not install git-remote-testgit Felipe Contreras
2013-06-09 16:40 ` [PATCH v4 03/45] build: trivial cleanup Felipe Contreras
2013-06-09 17:17   ` SZEDER Gábor
2013-06-09 16:40 ` [PATCH v4 04/45] build: add builtin lib Felipe Contreras
2013-06-09 16:40 ` [PATCH v4 05/45] log-tree: remove dependency from sequencer Felipe Contreras
2013-06-09 16:40 ` [PATCH v4 06/45] Move sequencer to builtin Felipe Contreras
2013-06-09 17:02   ` Antoine Pelisse
2013-06-09 17:06     ` Felipe Contreras
2013-06-09 17:07     ` Ramkumar Ramachandra
2013-06-09 17:10       ` Antoine Pelisse
2013-06-09 16:40 ` [PATCH v4 07/45] unpack-trees: plug a memory leak Felipe Contreras
2013-06-09 16:40 ` [PATCH v4 08/45] read-cache: plug a few leaks Felipe Contreras
2013-06-09 16:40 ` [PATCH v4 09/45] sequencer: remove useless indentation Felipe Contreras
2013-06-09 18:17   ` Fredrik Gustafsson
2013-06-09 18:19     ` Felipe Contreras
2013-06-09 19:08       ` Fredrik Gustafsson
2013-06-09 19:16         ` Felipe Contreras
2013-06-09 19:48           ` Fredrik Gustafsson
2013-06-09 16:40 ` [PATCH v4 10/45] sequencer: trivial fix Felipe Contreras
2013-06-09 17:18   ` SZEDER Gábor
2013-06-09 17:23     ` Felipe Contreras
2013-06-09 17:33       ` SZEDER Gábor
2013-06-09 17:37         ` John Keeping
2013-06-09 17:53           ` Felipe Contreras
2013-06-09 19:01             ` John Keeping [this message]
2013-06-09 19:11               ` Felipe Contreras
2013-06-09 19:20                 ` SZEDER Gábor
2013-06-09 17:47         ` Felipe Contreras
2013-06-09 17:53           ` SZEDER Gábor
2013-06-09 18:21             ` Felipe Contreras
2013-06-09 21:09       ` Philip Oakley
2013-06-09 16:40 ` [PATCH v4 11/45] cherry-pick: don't barf when there's nothing to do Felipe Contreras
2013-06-09 19:21   ` Fredrik Gustafsson
2013-06-09 19:32     ` Felipe Contreras
2013-06-09 16:40 ` [PATCH v4 12/45] cherry-pick: add --skip-empty option Felipe Contreras
2013-06-09 16:40 ` [PATCH v4 13/45] revert/cherry-pick: add --quiet option Felipe Contreras
2013-06-09 16:40 ` [PATCH v4 14/45] revert/cherry-pick: add --skip option Felipe Contreras
2013-06-09 16:40 ` [PATCH v4 15/45] builtin: add rewrite helper Felipe Contreras
2013-06-09 16:40 ` [PATCH v4 16/45] cherry-pick: store rewritten commits Felipe Contreras
2013-06-09 16:40 ` [PATCH v4 17/45] cherry-pick: don't store skipped commit Felipe Contreras
2013-06-09 16:40 ` [PATCH v4 18/45] builtin: move run_rewrite_hook() to rewrite.c Felipe Contreras
2013-06-09 16:40 ` [PATCH v4 19/45] builtin: add copy_rewrite_notes() Felipe Contreras
2013-06-09 16:40 ` [PATCH v4 20/45] cherry-pick: copy notes and run hooks Felipe Contreras
2013-06-09 17:22   ` Thomas Rast
2013-06-09 17:29     ` Felipe Contreras
2013-06-09 16:40 ` [PATCH v4 21/45] cherry-pick: add --action-name option Felipe Contreras
2013-06-09 16:40 ` [PATCH v4 22/45] cherry-pick: remember rerere-autoupdate Felipe Contreras
2013-06-09 16:40 ` [PATCH v4 23/45] rebase: split the cherry-pick stuff Felipe Contreras
2013-06-09 16:40 ` [PATCH v4 24/45] rebase: cherry-pick: fix mode storage Felipe Contreras
2013-06-09 16:40 ` [PATCH v4 25/45] rebase: cherry-pick: fix sequence continuation Felipe Contreras
2013-06-09 16:40 ` [PATCH v4 26/45] rebase: cherry-pick: fix abort of cherry mode Felipe Contreras
2013-06-09 16:40 ` [PATCH v4 27/45] rebase: cherry-pick: fix command invocations Felipe Contreras
2013-06-09 16:40 ` [PATCH v4 28/45] rebase: cherry-pick: fix status messages Felipe Contreras
2013-06-09 16:40 ` [PATCH v4 29/45] rebase: cherry-pick: automatically commit stage Felipe Contreras
2013-06-09 16:40 ` [PATCH v4 30/45] rebase: cherry-pick: set correct action-name Felipe Contreras
2013-06-09 16:40 ` [PATCH v4 31/45] rebase: trivial cleanup Felipe Contreras
2013-06-09 19:15   ` Fredrik Gustafsson
2013-06-11 16:12     ` Junio C Hamano
2013-06-11 17:08       ` Fredrik Gustafsson
2013-06-11 17:09         ` Felipe Contreras
2013-06-11 17:24           ` Fredrik Gustafsson
2013-06-11 17:26             ` Felipe Contreras
2013-06-11 17:41               ` Fredrik Gustafsson
2013-06-11 17:42                 ` Felipe Contreras
2013-06-09 16:40 ` [PATCH v4 32/45] rebase: use 'cherrypick' mode instead of 'am' Felipe Contreras
2013-06-09 16:40 ` [PATCH v4 33/45] rebase: cherry-pick: fix for shell prompt Felipe Contreras
2013-06-09 16:40 ` [PATCH v4 34/45] rebase: cherry-pick: add merge options Felipe Contreras
2013-06-09 16:40 ` [PATCH v4 35/45] rebase: remove merge mode Felipe Contreras
2013-06-09 16:40 ` [PATCH v4 36/45] rebase: cherry-pick: add copyright Felipe Contreras
2013-06-09 16:40 ` [PATCH v4 37/45] tests: fix autostash Felipe Contreras
2013-06-09 16:40 ` [PATCH v4 38/45] add simple tests of consistency across rebase types Felipe Contreras
2013-06-09 16:40 ` [PATCH v4 39/45] add tests for rebasing with patch-equivalence present Felipe Contreras
2013-06-09 16:40 ` [PATCH v4 40/45] add tests for rebasing of empty commits Felipe Contreras
2013-06-09 16:40 ` [PATCH v4 41/45] add tests for rebasing root Felipe Contreras
2013-06-09 16:40 ` [PATCH v4 42/45] add tests for rebasing merged history Felipe Contreras
2013-06-09 17:25   ` Thomas Rast
2013-06-09 17:31     ` Felipe Contreras
2013-06-09 17:32       ` Thomas Rast
2013-06-09 17:46         ` Felipe Contreras
2013-06-09 16:40 ` [PATCH v4 43/45] t3406: modernize style Felipe Contreras
2013-06-09 16:40 ` [PATCH v4 44/45] tests: move test for rebase messages from t3400 to t3406 Felipe Contreras
2013-06-09 16:40 ` [PATCH v4 45/45] tests: update topology tests Felipe Contreras
2013-06-11  4:37   ` Martin von Zweigbergk
2013-06-11  4:41     ` Felipe Contreras

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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=20130609190103.GG22905@serenity.lan \
    --to=john@keeping.me.uk \
    --cc=artagnon@gmail.com \
    --cc=felipe.contreras@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jrnieder@gmail.com \
    --cc=martin.von.zweigbergk@gmail.com \
    --cc=szeder@ira.uka.de \
    /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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).