All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Koosha Khajehmoogahi <koosha@posteo.de>
Cc: git@vger.kernel.org
Subject: Re: [PATCH v2 2/5] log: honor log.merges= option
Date: Sun, 12 Apr 2015 21:56:26 -0700	[thread overview]
Message-ID: <xmqqoams4oed.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <5525062A.2010905@posteo.de> (Koosha Khajehmoogahi's message of "Wed, 08 Apr 2015 12:42:50 +0200")

Koosha Khajehmoogahi <koosha@posteo.de> writes:

> On 04/08/2015 04:28 AM, Junio C Hamano wrote:
>
>> It is strange that you have to ask me to give you the reason why you
>> chose it that way, isn't it?
>
> AFAIK, the only other command that supports --merges and --no-merges options is
> rev-list. This new feature aims to make a default behavior for the commands
> that have these options. The command-line option is supported by the two commands.
> However, the config var is only used by git-log and rev-list ignores it. I didn't
> exclude rev-list for any particular reason. If we need, I could also handle it in
> rev-list.

As rev-list is plumbing, it shouldn't be affected by the UI level
customization knobs like this one; otherwise you will break people's
scripts when end users choose to use the new knobs.

Historically, "whatchanged" has been the way to ask for "log", with
different default output format (but not different commit selection
logic).  I would think people who are used to "whatchanged" would
expect that the command would pay attention to what "log" would.

As I already mentioned with the reason why, I do not think "show"
and "format-patch" should pay attention to it.

There may be other commands from the "log" family (i.e. what is
defined in builtin/log.c and/or uses get_revision() API to walk the
commit graph), for which similar reasoning should be done to decide
if each of them should or should not pay attention to it.

Thanks.

  reply	other threads:[~2015-04-13  4:56 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <266077>
2015-04-04  1:21 ` [PATCH v2 1/5] revision: add --merges={show|only|hide} option Koosha Khajehmoogahi
2015-04-04  1:21   ` [PATCH v2 2/5] log: honor log.merges= option Koosha Khajehmoogahi
2015-04-04 20:00     ` Junio C Hamano
2015-04-07 22:15       ` Koosha Khajehmoogahi
2015-04-08  2:28         ` Junio C Hamano
2015-04-08 10:42           ` Koosha Khajehmoogahi
2015-04-13  4:56             ` Junio C Hamano [this message]
2015-04-07  5:18     ` Eric Sunshine
2015-04-04  1:21   ` [PATCH v2 3/5] Documentation: add git-log --merges= option and log.merges config. var Koosha Khajehmoogahi
2015-04-05 21:41     ` Junio C Hamano
2015-04-04  1:22   ` [PATCH v2 4/5] t4202-log: add tests for --merges= Koosha Khajehmoogahi
2015-04-07  7:32     ` Eric Sunshine
2015-04-04  1:22   ` [PATCH v2 5/5] bash-completion: add support for git-log --merges= and log.merges Koosha Khajehmoogahi
2015-04-07  5:16   ` [PATCH v2 1/5] revision: add --merges={show|only|hide} option Eric Sunshine

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=xmqqoams4oed.fsf@gitster.dls.corp.google.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=koosha@posteo.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.