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
smime.p7s
Description: S/MIME cryptographic signature
