git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "Miles Bader" <miles@gnu.org>
Cc: "Junio C Hamano" <gitster@pobox.com>, git@vger.kernel.org
Subject: Re: [PATCH] Make git-archimport log entries more consistent
Date: Wed, 29 Aug 2007 21:10:57 -0700	[thread overview]
Message-ID: <7vir6xacha.fsf@gitster.siamese.dyndns.org> (raw)
In-Reply-To: <fc339e4a0708292019s3d4f6914h4f9efe6f1172c380@mail.gmail.com> (Miles Bader's message of "Thu, 30 Aug 2007 12:19:00 +0900")

"Miles Bader" <miles@gnu.org> writes:

> On 8/30/07, Junio C Hamano <gitster@pobox.com> wrote:
>> > This patch makes git-archimport generate one blank line as a separator in all
>> > cases.
>>
>> I would not have any problem with what the result of this patch
>> would record in the commits, if it was what it did from the very
>> beginning.  But this is a change in behaviour; I'd like to know
>> if people who use archimport _rely_ on the current behaviour...
>
> Good point, though it seems pretty unlikely -- the most notable thing
> about the old behavior was that the results were inconsistent... :-)

I think the "consistency" is debatable.  If somebody was parsing
them mechanically, the original code did:

     print WRITER $ps->{summary},"\n\n";
-    print WRITER $ps->{message},"\n";
     print WRITER 'git-archimport-id: ',$ps->{id},"\n";

which means the program can read the last line to get id, go
back to find "\n\n" and treat the one before it as summary, and
take the rest which could be empty as message.  That's also
consistent.

After your change, 

     print WRITER $ps->{summary},"\n\n";
+    print WRITER $ps->{message},"\n\n" if ($ps->{message} ne "");
     print WRITER 'git-archimport-id: ',$ps->{id},"\n";

the last line is id, go back to find "\n\n" and use the one
before it as summary, and take the rest as message but (1) if
the message is not empty, it is followed by "\n\n" so the last
"\n" needs to be stripped, (2) otherwise it is not.

I do not think anybody would doubt that the updated one is nicer
to the eye.  That's why I said it would have been nicer if the
message was formatted like that from the beginning.  But I do
not think you can claim it is _more_ consistent.  It just
formats under a rule different from the original.  The issue is
if the "nicer-to-the-eye" outweighs potential breakage the
reformatting can cause to existing parsers, if any.

That's why I wanted to know if people _rely_ on the current
behaviour, because I was hoping that everybody would answer "yes
nicer-to-the-eye is more important and there is no drawback".

  reply	other threads:[~2007-08-30  4:11 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-08-30  1:56 [PATCH] Make git-archimport log entries more consistent Miles Bader
2007-08-30  3:14 ` Junio C Hamano
2007-08-30  3:19   ` Miles Bader
2007-08-30  4:10     ` Junio C Hamano [this message]
2007-08-30  7:47       ` David Kastrup
2007-08-30 21:46         ` Junio C Hamano
  -- strict thread matches above, loose matches on Subject: below --
2007-08-16  5:01 Miles Bader

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=7vir6xacha.fsf@gitster.siamese.dyndns.org \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=miles@gnu.org \
    /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).