Re: [Qemu-devel] why restrict pull reqs to signed tags?

[David Woodhouse <dwmw2@infradead.org>]

[-- Attachment #1: Type: text/plain, Size: 4282 bytes --]

On Wed, 2016-03-09 at 13:13 +0100, Laszlo Ersek wrote:
> I understand, thank you. Especially your "directly commit to master"
> analogy is good. Pulling replaces your detailed personal review with the
> trusted identity of the pull requestor -- you trust that the commits on
> the requestor's branch are already sufficiently reviewed.

Note that it doesn't *have* to. And again, there's nothing special
about email vs. pull requests here.

Pater is saying that he *chooses* not to bother reviewing what he pulls
in via pull requests, and *that's* why it's equivalent to direct commit
access.

I could just as well say that *I* choose to hold my nose and look the
other way while running 'git am', and thus *patches* would be
equivalent to direct commit access.

I won't tell Peter that his behaviour is wrong. I'll only say that
other projects don't have to do the same thing.

And repeat that from the trust point of view, there is *nothing*
fundamentally different about patches vs. pull requests.

> http://thread.gmane.org/gmane.linux.kernel/1855303/focus=2172988
> 
> > 
> > Conversely, a random set of patches sent to the list is supposed
> > to be reviewed and tested by the submaintainer who applies them to
> > their tree -- that is the gateway at which review happens.
> This was my understanding, yes.
> 
> David is proposing that direct pull requests be allowed on edk2-devel,
> immediately from contributors, so that the contributor may ask for
> his/her exact history to be preserved. I'm looking for examples: high
> profile projects that have adopted such a workflow *all the while*
> enforcing patch-wise reviews. Thus far I've come up empty.
>
> I think the idea we have thus far is:
> 
> - submitter posts the patches
> - patches are reviewed on the list
> - submitter picks up the R-b, A-b, T-b labels

 (as well as any other feedback, of course)

> - when converged, submitter sends a pull request with the labels
> applied, with the history he or she likes
> - maintainer fetches the branch, verifies that the commits indeed match
> the patches on list; also verifies that the labels have been correctly
> picked up from the list

 All of which the maintainer is already expected to do, right? Except
instead of 'fetches the branch' the maintainer is currently applying
the patches in his/her *own* mailbox, potentially to a current master
on which they don't actually work, and then doing the rest of what you
said.

> - maintainer merges the branch locally and pushes the merge commit (and
> its deps) to upstream master

Well, the above two steps would be 'pull, then look'. I don't think
explicit messing with topic branches and manual merges would be
necessary.

You do a 'git pull $SUBMITTED_TREE'. If you like what you get, you then
just do a 'git push'. If you don't, 'git reset --hard origin' and send
an email saying why it was rejected.


> I feel a bit uncertain that we're trailblazing this workflow. It could
> work I guess.

We wouldn't be trailblazing it at all. It's done all the time in Linux
and various other projects. It's just that the 'submitter' rôle is
split between individual contributors, and subsystem maintainers.

> - submitter posts the patches
> - patches are reviewed on the list
> - submitter picks up the R-b, A-b, T-b labels

In fact either the submitter will pick them up when sending a second
round of patches (if there are any *other* changes to make), or the
subsystem maintainer will pick them up (via patchwork, usually) when
applying the patches to the subsystem tree.

> - when converged, submitter sends a pull request with the labels
applied, with the history he or she likes

The subsystem maintainer sends the pull request to Linus.

> - maintainer fetches the branch, verifies that the commits indeed match
> the patches on list; also verifies that the labels have been correctly
> picked up from the list
> - maintainer merges the branch locally and pushes the merge commit
> (and its deps) to upstream master

Linus does a test pull, and either likes what he sees, or doesn't. Or
depending on who it comes from and how much he cared about the code
being modified, doesn't look too hard.


-- 
dwmw2


[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5691 bytes --]