netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: "Michael S. Tsirkin" <mst@redhat.com>,
	KVM list <kvm@vger.kernel.org>,
	virtualization <virtualization@lists.linux-foundation.org>,
	Network Development <netdev@vger.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	jmarchan@redhat.com, Kamal Mostafa <kamal@canonical.com>,
	Sasha Levin <sasha.levin@oracle.com>
Subject: Re: [PULL] virtio: fixes, tests
Date: Wed, 27 Jan 2016 22:26:13 -0800	[thread overview]
Message-ID: <xmqqd1sm5iey.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <CA+55aFzC6mopLkYYvuZNK8W510MpqgSuYwhjAAM-XeuAdEfs3A@mail.gmail.com> (Linus Torvalds's message of "Wed, 27 Jan 2016 14:57:28 -0800")

Linus Torvalds <torvalds@linux-foundation.org> writes:

> On Wed, Jan 27, 2016 at 2:15 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
>>>
>>> Anyway, pulled. Just curious about how that thing happened.
>>
>> That's because apparently diffstat obeys orderfile rules:
>
> Ugh. I guess that makes sense, but it's still very annoying for
> something like a pull request, where now different people end up
> having different diffstats. And the reason I never noticed it is that
> likely there aren't that many people who use an orderfile.
>
> I guess something like "-O /dev/null" in the pull-request would undo
> it, but it is a bit annoying.
>
> I've never actually met anybody (knowingly) that used that option. I
> thought it was a Junio-only use case (it's been around forever as a
> command line option, but the config file entry seems to be somewhat
> recent and I wasn't even aware of it).
>
> Adding Junio just as background to see what he thinks. Looks like the
> diff.orderfile config option hits not just porcelain, but plumbing
> too.

Note: without enough context, I am guessing that you are annoyed
that diffstat graph in a pull request did not match what you got
locally after pulling.

I've never actually met anybody who uses the orderfile, either.  And
I tend to agree that it probably is a mistake if the configuration
is not limited to diff_ui_config().

Wait.  It _is_ limited to diff_ui_config().  The thing is, "git
request-pull" does use the Porcelain "git diff" to emit the final
diffstat.  So there is nothing to fix in diff.c, at least ;-)

I am of two minds about the configuration affecting the output from
"git request-pull" command as a whole.  In a sense, "request-pull"
command itself _is_ a UI level thing, i.e. Porcelain, and if a
project chooses to standardize on a certain patch/diff presentation
order using the diff.orderfile facility, there should be a way to
tell your project participants to use the same order when they send
in their patches and their pull requests, but with the hardcoded
"git diff --stat" invocation at the end of "git request-pull", such
a per-project customization must happen via _some_ configuration
variable.  We _could_ make up request-pull.difforderfile that is
read by that script and gets turned into "-O $file" arguments to the
"git diff" invocation, but if projects really wants to standardize
on a single patch/diff presentation order, I do not think such a
split configuration buys us anything.  We'd be better off using a
single diff.orderfile and have it consistently honoured by tools
everybody who participates in the project uses.

You obviously can declare "With MY project, the standard order in
which you must present patch/diff is with this empty file", and tell
your project participants to set diff.orderfile to /dev/null in the
repository config when they work on the kernel.  That would be what
any such hypothetical project would do, when it has a specific
preference of the patch/diff presentation order.  After all, you do
have a specific preference, which is "do not futz with the order in
which Git gives the paths by default", so...

      reply	other threads:[~2016-01-28  6:26 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-27 19:12 [PULL] virtio: fixes, tests Michael S. Tsirkin
2016-01-27 19:57 ` Linus Torvalds
2016-01-27 22:15   ` Michael S. Tsirkin
2016-01-27 22:57     ` Linus Torvalds
2016-01-28  6:26       ` Junio C Hamano [this message]

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=xmqqd1sm5iey.fsf@gitster.mtv.corp.google.com \
    --to=gitster@pobox.com \
    --cc=jmarchan@redhat.com \
    --cc=kamal@canonical.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mst@redhat.com \
    --cc=netdev@vger.kernel.org \
    --cc=sasha.levin@oracle.com \
    --cc=torvalds@linux-foundation.org \
    --cc=virtualization@lists.linux-foundation.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).