All of lore.kernel.org
 help / color / mirror / Atom feed
From: Michael J Gruber <git@drmicha.warpmail.net>
To: Stefan Beller <sbeller@google.com>,
	"H. Peter Anvin" <hpa@zytor.com>,
	Git Mailing List <git@vger.kernel.org>
Cc: Junio C Hamano <gitster@pobox.com>,
	ebiederm@xmission.com, Fengguang Wu <fengguang.wu@intel.com>,
	Xiaolong Ye <xiaolong.ye@intel.com>,
	"git@vger.kernel.org" <git@vger.kernel.org>,
	ying.huang@intel.com, philip.li@intel.com, julie.du@intel.com,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Christoph Hellwig <hch@lst.de>,
	Dan Carpenter <dan.carpenter@oracle.com>,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: [RFC/PATCH 1/1] format-patch: add an option to record base tree info
Date: Wed, 24 Feb 2016 11:31:29 +0100	[thread overview]
Message-ID: <56CD8681.9010308@drmicha.warpmail.net> (raw)
In-Reply-To: <CAGZ79kYVHJFeS41yj+JymKyfKpSW4y-Wpk6ZTT4yxzprC6UQTg@mail.gmail.com>

Stefan Beller venit, vidit, dixit 23.02.2016 23:21:
> On Tue, Feb 23, 2016 at 12:46 PM, H. Peter Anvin <hpa@zytor.com> wrote:
>> On February 23, 2016 12:35:12 PM PST, Junio C Hamano <gitster@pobox.com> wrote:
>>> ebiederm@xmission.com (Eric W. Biederman) writes:
>>>
>>>> Junio C Hamano <gitster@pobox.com> writes:
>>>>
>>>>> It is valuable for a testing organization to say "We tested this
>>>>> series on top of version X.  We know it works, we have tested on a
>>>>> lot more hardware than the original developer had, we know this is
>>>>> good to go."  It is a valuable service.
>>>>>
>>>>> But that is valuable only if version X is still relevant, isn't it?
>>>>>
>>>>> Is the relevance of a version something that is decided by a
>>>>> developer who submits a patch series, or is it more of an attribute
>>>>> of the project and where the current integration is happening?
>>>>> Judging from the responses from Dan to this thread, I think the
>>>>> answer is the latter, and for the purpose of identifying the
>>>>> relevant version(s), the project does not even care about the exact
>>>>> commit, but it wants to know more about which branch the series is
>>>>> targetted to.
>>>>>
>>>>> With that understanding, I find it hard to believe that it buys the
>>>>> project much for the "base" commit to be recorded in a patch series
>>>>> and automated testing is done by applying the patches to that exact
>>>>> commit, which possibly is no-longer-relevant, even though it may
>>>>> help shielding the testing machinery from "you tested with a wrong
>>>>> version" complaints.
>>>>>
>>>>> Isn't it more valuable for the test robot to say "this may or may
>>>>> not have worked well with whatever old version the patch series was
>>>>> based on, but it no longer is useful to the current tip of the
>>>>> 'master'"?  If you consider what benefit the project would gain by
>>>>> having such a robot, that is the conclusion I have to draw.
>>>>>
>>>>> So I still am not convinced that this "record base commit" is a
>>>>> useful thing to do.
>>>>
>>>> So I don't know what value this has to the git project.
>>>
>>> Oh, don't worry, I wasn't talking about value this may have to the
>>> Git project at all.  "The value to the project" I mentioned in my
>>> response was all about the value to the kernel project.
>>>
>>>> The value of Fengguag's automated testing to the kernel project is to
>>>> smoke out really stupid things.
>>>
>>> I'll snip your bullet points, but as I wrote, I do understand the
>>> value of prescreening test.
>>>
>>> What I was questioning was what value it gives to that testing to
>>> use "the developer based this patch on this exact commit" added to
>>> each incoming patch, and have Fengguag's testing machinery test a
>>> patch with a base version that may no longer be relevant in the
>>> context of the project.  Compared to that, wouldn't "this no longer
>>> applies to the branch the series targets" or "this still applies
>>> cleanly, but no longer compiles because the surrounding API has
>>> changed" be much more valuable?
>>>
>>> In your other message, you mentioned the "index $old..$new" lines,
>>> and it is possible to use them to find a tree that the patch cleanly
>>> applies to, but it will not uniquely identify _the_ version the
>>> patch submitter used.  IMHO, finding such _a_ tree from the recent
>>> history of the branch that the patch targets and testing the patch
>>> on top of that tree (as opposed to testing the patch in the exact
>>> context the developer worked on) would give the project a better
>>> value.
>>
>> Personally, as a maintainer, I would love to see the tree ID and ideally also the commit ID a series is based on.  The commit ID is in some ways less useful since it is non-recreatable (and therefore will never match for anything but the first patch of a series), but could be useful to the maintainer.
> 
> As a contributor a commit id would also add value I would think. When
> it is unclear
> where a series is headed, contributors in Git land say things like:
> 
>     This applies cleanly on origin/master.
> 
> And usually this is the master branch from yesterday as Junio pushes
> once a day. origin/master being a moving target, so it may not apply any more,
> so a commit sha1 would help for finding out what to do in the maintainer role.
> 
> This discussion sounds to me as if we'd want to have some advantages of
> the (kernel pull style, not github style) pull-request here for patch
> submissions.
> 
> I don't remember the exact quote, but Linus said once upon a time about the
> pull request workflow roughly:
> 
>     "Please pull from ... And by the way I tested this software for 2
> month during development"
>     (For kernels that makes sense as the contributor run the kernel
> and it worked)
> 
> as pull requests have the new patches on top of the exact parents the
> contributor put them
> on, there can be more faith in the process to divide between the
> problems the contributor
> overlooked/introduced and problems as introduced by the merge of the maintainer.
> 
> Now when applying patches at another parent than the contributor
> developed on, some
> subtle problems may arise, which are not easily spotted by compile
> tests or running the
> test suite.
> 
> Maybe this could be solved by a convention (similar to a sign-off line
> in each patch
> which is not formally checked) in the cover letter, such that we have
> a both machine
> and human readable format where the contributor can suggest an anchor point?
> (The maintainer may ignore it, but buildbots are free to follow strictly)
> 
> Thanks,
> Stefan

Thanks for mentioning pull-requests. In fact: Is there any problem (in
the OP's use case) that would not be solved by pull-requests?

In other words: Are we talking "pull-request by format-patch"?

[Still trying to figure out what problem to solve exactly, and how
generic that is.]

Michael

  reply	other threads:[~2016-02-24 10:31 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-02-22  2:58 [RFC/PATCH 0/1] Add an option to git-format-patch to record base tree info Xiaolong Ye
2016-02-22  2:58 ` [RFC/PATCH 1/1] format-patch: add an option " Xiaolong Ye
2016-02-22  4:19   ` Junio C Hamano
2016-02-22  7:30     ` Jacob Keller
2016-02-23  1:47     ` Fengguang Wu
2016-02-23  6:54       ` Junio C Hamano
2016-02-23  9:17         ` Fengguang Wu
2016-02-23  9:23           ` H. Peter Anvin
2016-02-23  9:32             ` Fengguang Wu
2016-02-23 10:32           ` Dan Carpenter
2016-02-23 12:00             ` Fengguang Wu
2016-02-23 13:31               ` Dan Carpenter
2016-02-24  2:55                 ` Fengguang Wu
2016-02-24  6:30                   ` Junio C Hamano
2016-02-24  7:07                     ` Fengguang Wu
2016-02-24 18:34                       ` Junio C Hamano
2016-02-23 19:51           ` Junio C Hamano
2016-02-23 20:08             ` Eric W. Biederman
2016-02-23 20:35               ` Junio C Hamano
2016-02-23 20:46                 ` H. Peter Anvin
2016-02-23 21:49                   ` Eric W. Biederman
2016-02-24  1:40                     ` H. Peter Anvin
2016-02-23 22:21                   ` Stefan Beller
2016-02-24 10:31                     ` Michael J Gruber [this message]
2016-02-24  6:19                   ` Junio C Hamano
2016-02-24  3:36                 ` Fengguang Wu
2016-02-24  3:13             ` Fengguang Wu
2016-02-23 19:56           ` Eric W. Biederman
2016-02-24  2:30             ` Fengguang Wu

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=56CD8681.9010308@drmicha.warpmail.net \
    --to=git@drmicha.warpmail.net \
    --cc=dan.carpenter@oracle.com \
    --cc=ebiederm@xmission.com \
    --cc=fengguang.wu@intel.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=hch@lst.de \
    --cc=hpa@zytor.com \
    --cc=julie.du@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=philip.li@intel.com \
    --cc=sbeller@google.com \
    --cc=torvalds@linux-foundation.org \
    --cc=xiaolong.ye@intel.com \
    --cc=ying.huang@intel.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 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.