git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org, kernel@pengutronix.de
Subject: Re: [PATCH RFC] log-tree: let format-patch not indent notes
Date: Thu, 25 Sep 2014 20:08:32 +0200	[thread overview]
Message-ID: <20140925180832.GA31554@pengutronix.de> (raw)
In-Reply-To: <xmqqeguzboka.fsf@gitster.dls.corp.google.com>

Hello Junio,

On Thu, Sep 25, 2014 at 10:24:53AM -0700, Junio C Hamano wrote:
> Uwe Kleine-König  <u.kleine-koenig@pengutronix.de> writes:
> > Commit logs as shown by git-log are usually indented by four spaces so
> > here it makes sense to do the same for commit notes.
> >
> > However when using format-patch to create a patch for submission via
> > e-mail the commit log isn't indented and also the "Notes:" header isn't
> > really useful. So consequently don't indent and skip the header in this
> > case. This also removes the empty line between the end-of-commit marker
> > and the start of the notes.
> >
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > ---
> > This commit changes the output of format-patch (applied on this commit) from:
> >
> > 	...
> > 	case.
> >
> > 	Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > 	---
> >
> > 	Notes:
> > 	    This commit changes the output of format-patch (applied on this commit) from:
> >
> > to
> >
> > 	...
> > 	case.
> >
> > 	Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > 	---
> > 	This commit changes the output of format-patch (applied on this commit) from:
> >
> > which I consider to be more useful.
> 
> I suspect that is fairly subjective, as the current one is in that
> form because those who wrote this feature first, reviewed, applied
> would have considered it more useful, isn't it?
Well, I thought when the feature to dump the notes into a patch was
created there was exactly one way these notes were written. This was was
designed for git-log and so intended and with "Notes:". For
git-format-patch it was good enough.

> Because I never send out a format-patch output without looking it
> over in an editor, I know I can easily remove it if I find the
> "Notes:" out of place in the output, but if the "Notes:" thing
> weren't there in the first place I may scratch my head trying to
> figure out where to update it if the information there were stale,
> so for that reason I'd find it more useful to have Notes: to remind
> me where that information comes from.
As you must explicitly request notes to be included in patches (--notes)
I think it's unusual to not know where the info comes from, doesn't it?

I don't know how many people use git-notes to track their comments, but
the first thing I do when editing patches is to remove the Notes: header
and s/^    // on the remaining lines. And most of the time this is the
only thing I do and I need to touch every patch only because of
that.

> But that is just my personal preference and I am willing to be
> persuaded either way with a better argument than "to me it looks
> nicer".
> 
> As to indenting, because the material after three-dashes is meant to
> be fed to "git apply" or "patch", I'd prefer to keep it to avoid
> having to worry about a payload that may look like part of a patch.
> This preference is a bit stronger than the presence/absence of
> "Notes:".
Ok, that's a valid concern. If we want to assert that this doesn't look
like a patch we need to at least parse the notes and quote it somehow.
Hmm.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

  reply	other threads:[~2014-09-25 18:08 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-09-25 16:10 [PATCH RFC] log-tree: let format-patch not indent notes Uwe Kleine-König
2014-09-25 17:24 ` Junio C Hamano
2014-09-25 18:08   ` Uwe Kleine-König [this message]
2014-09-25 17:56 ` Jeff King

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=20140925180832.GA31554@pengutronix.de \
    --to=u.kleine-koenig@pengutronix.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=kernel@pengutronix.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.
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).