About an IRC discussion on using pull requests for hg development

[Matt Mackall]

On Tue, May 08, 2012 at 11:23:30AM +0200, Martin Geisler wrote:
> Matt Mackall <mpm at selenic.com> writes:
> 
> > On Mon, 2012-05-07 at 18:33 +0200, Patrick Mézard wrote:
> >> TL;DR:
> >> - Make history editing easier and safer (include histedit?)
> >> - Vague thoughts about supporting bitbucket-like pull requests while
> >> preserving an email-based review workflow.
> >
> > Every time this comes up, I say "but I do accept pull requests!"
> >
> > But I only want pull requests from people who have a track record of
> > getting a submission right the first time and have internalized
> > important rules like "don't mix unrelated changes". Otherwise I'll
> > start reading your incoming csets, discover problems, then ask you to
> > resend so I can comment on the problems. Which means my aborted first
> > pass was wasted effort.
> 
> Yeah, I understand why this workflow requires nicely separated patches.
> I also agree that clean history is very valuable. I've been annoyed
> several times by now because largefiles were committed as one big
> changeset by that "Various" guy... he cannot be be trusted to write bug
> free code :)

Well that's actually not the point I wanted you to come away with.
What's most important is: review must happen on the mailing list. The reason I
don't want to see patches on the BTS and the reason I don't want to
pull from random people is the same: I want review to happen in a
public place where a maximum number of people can contribute and
benefit from review comments, and everything is searchably archived.

Since review must happen on the list, and everything should be
reviewed, code submission needs to happen on the list. And raising the
bar for starting a review above hitting reply is a big step backward.

(Also remember that it's handy for me to be able to cherrypick
submissions and send pieces back for reworking.)

> Maybe the problem is that we focus on the individual changesets and not
> on the succesion of branch heads?
> 
> That is, maybe 'hg push' or 'hg pull' could add a marker to the new
> changegroup saying "these changesets came in together". Things like
> bisect could then operate on a changegroup level, maybe TortoiseHg would
> collapse the changegroups by default, ...
> 
> The result might be a system where it's okay to use 5 half-broken
> commits to implement 1 new feature. I imagine that it would be possible
> to add more such group markers locally so that someone could push two
> groups to Bitbucket and ask you to pull them.
> 
> When you use pull requests in Bitbucket you don't see the diffs in the
> individual changesets: you see the combined diff. So that already
> encourages this view: if you're not happy with the diff, then you ask
> the submitter to reviese the pull request and he can do that by *adding*
> new changesets. Similarly to how we normally advertise 'hg backout' as
> the way to fix mistakes.

That actually seems quite counter to what I want: small, obviously
correct, reviewable changes. If you make five small changes, test
them, amend them with three more cleanup commits, and then collapse
them to one grouped change.. you might as well have done one fat
commit for all the good it does me.

Also such grouped commits will surely contain broken intermediate
commits that mean they'll be the bane of anyone trying to bisect.

But at any rate, the future is Pierre-Yves "evolution" scheme. If you
really want to see this workflow improve, you should be testing his
code and figuring out how to best integrate it with email.

-- 
Mathematics is the supreme nostalgia of our time.