git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jonathan Nieder <jrnieder@gmail.com>
To: Jay Soffian <jaysoffian@gmail.com>
Cc: Junio C Hamano <gitster@pobox.com>, git@vger.kernel.org
Subject: Re: [PATCH v3] Add log.abbrevCommit config variable
Date: Mon, 16 May 2011 02:00:47 -0500	[thread overview]
Message-ID: <20110516070047.GA26270@elie> (raw)
In-Reply-To: <BANLkTi=iFgJ12=_amoxT8x+hNMEhQtOVBg@mail.gmail.com>

Jay Soffian wrote:
> On Sun, May 15, 2011 at 6:42 PM, Junio C Hamano <gitster@pobox.com> wrote:

>> I am not entirely happy about this change. The "ditto" refers to an ugly
>> workaround we had to add with 4f62c2b (log.decorate: only ignore it under
>> "log --pretty=raw", 2010-04-08) only because it was too late to revert the
>> change in 72d9b22 (Merge branch 'sd/log-decorate', 2010-05-08) that was
>> about to become part of 1.7.2-rc0 release. If we knew better, we probably
>> wouldn't have added the log.decorate variable that requires this hack that
>> requires us to say that 'log --pretty=raw' is special.
>>
>> If we stop before adding a new configuration, we do not have to repeat the
>> same mistake we made earlier.
>>
>> I dunno.
>
> I disagree that log.decorate is a mistake and that the workaround is
> ugly.

I suppose part of what Junio is saying is that by the time the commits
referenced above were written, git had already broken some scripts
(including gitk) and those changes were part of a desparate attempt to
contain the damage.  So they are not a great example to look to for
the sort of smooth transition it is possible to set up proactively.

For example, maybe (after fixing the scripts we already know about,
such as tig) we could add the log.abbrevcommit variable right away but
advertise it as experimental:

	*Warning* This option is experimental and will break your
	scripts.  It is only provided to give script authors a
	chance to test this functionality and fix their scripts
	before the feature is advertised in earnest.

One transition plan could look like this:

 1. In the release notes to v1.7.6, mention that there is a change
    on the horizon that would break people's scripts and encourage
    script authors to switch to "rev-list | diff-tree -s --stdin"
    if their scripts depend on the details of "git log" format
    (in particular, if their scripts do not work correctly after
    s/log/log --abbrev-commit/).  Introduce the log.abbrevcommit
    variable to help people test, guarded by a compile-time
    option and disabled by default.

 2. In v1.7.7, introduce the log.abbrevcommit variable, advertised
    as "This will break your system --- don't use it unless you
    are trying to find such breakage and fix it".

 3. In v1.8.0, introduce the variable in earnest and recommend
    that people use it.

I think step 1 is going too far --- it should be possible to give
users rope like this without worrying that they are going to be
irresponsible about it.

Now, returning to "log --pretty=raw".  Is it plumbing or not?  It
would be nice to advertise whichever way it is decided (I guess it is
de facto plumbing) in the "git log" reference documentation and to
follow that decision in cases like this one.

Thanks for some food for thought.

Ciao,
Jonathan

  reply	other threads:[~2011-05-16  7:01 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-05-14 17:22 [PATCH] Add log.abbrev-commit config option Jay Soffian
2011-05-14 17:26 ` Jay Soffian
2011-05-14 19:01 ` Jonathan Nieder
2011-05-14 19:35   ` Jay Soffian
2011-05-14 20:19     ` [PATCH] add, merge, diff: do not use strcasecmp to compare config variable names Jonathan Nieder
2011-05-15  1:53       ` Junio C Hamano
2011-05-14 20:47   ` [PATCH v2] Add log.abbrevCommit config variable Jay Soffian
2011-05-14 21:55     ` Jonathan Nieder
2011-05-14 22:22       ` Jay Soffian
2011-05-14 22:49         ` [PATCH v3] " Jay Soffian
2011-05-15 13:25           ` Jay Soffian
2011-05-15 22:42           ` Junio C Hamano
2011-05-16  5:53             ` Jay Soffian
2011-05-16  7:00               ` Jonathan Nieder [this message]
2011-05-16  7:18                 ` Jay Soffian
2011-05-17 17:03                 ` Junio C Hamano
2011-05-17 21:50                   ` Junio C Hamano
2011-05-18  1:05                     ` Jay Soffian
2011-05-17 18:50           ` Junio C Hamano
2011-05-17 19:08             ` Jay Soffian
2011-05-15  1:48   ` [PATCH] Add log.abbrev-commit config option Junio C Hamano
2011-05-16  8:24     ` Michael J Gruber

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=20110516070047.GA26270@elie \
    --to=jrnieder@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jaysoffian@gmail.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).