About an IRC discussion on using pull requests for hg development

[Levi Bard]

>> > <mpm>
>> > 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.

Why?
If you want to have a hierarchy where incoming changes go through crew
(or someplace) first, meaning that they're reviewed/accepted/merged by
somebody that's not you, that's ok - but that's not what exists now.
So, after creating a patch, my options are:
 * Figure out wtf patchbomb is, argue with people for a while about
whether I should really have to use it in 2012, trial/error until I
get it configured correctly, send patch for review, receive comments,
decide how to rework patch, figure out wtf mq is, ...
 * GTFO

This is not attractive to me as a new contributor.

> <mpm>
> What's most important is: review must happen on the mailing list. [...]
> 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.

These conditions do not imply that conclusion.
I agree that review should happen transparently in public, and should
be archived and searchable. However, the mailing list is not the only
technology that has achieved these lofty goals.
A web review solution will serve at least as well, and activity should
actually be able to be mirrored between the web interface and the ML
without too much work.

> <mpm>
> 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.)

This conclusion also follows the flawed logic from the previous snippet.
A ML is a terrible forum for code review, and an even worse forum for
code submission. This is a workflow that requires every involved party
to have an ideally configured mail client for viewing and reviewing
code precisely the way the mercurial project does it and to have
adapted to the workflow of interacting with reviews of series of
patches in this way.
A web review system allows all the configuration to happen on the
server, and everyone gets the same interface to and experience with
the review.
I also agree that one should just be able to reply via mail to a
notification from the system, and have that aggregated into the web
view of the review. And if anything, it should be _easier_ to
cherrypick changesets from a repo and get the benefits of merge
machinery, etc., than to manually apply patches from a mail.

Benefits of a hosted review system over a ML:
 * Code and diff highlighting (for everyone)
 * Ability to transform the view of the changes being reviewed
(per-changeset, aggregate diff, etc.)
 * Ability to review non-text changes (largefiles?)
 * Ability to navigate to different changesets/source files/etc. via
hyperlinks directly from the review context
 * Familiarity for free software developers who have interacted with
Bitbucket, Launchpad, Github, and friends
 * The entire process is public and transparent from end to end:
   * Current process: Write patch => patchbomb to list => review =>
"queued" announcement => ??? => patch lands in hg repo, unless
something goes wrong
   * Web review process: Write patch => push => review =>
merged/rebased to hg repo

>> <mg>
>> 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, ...

This sounds to me like a merge. Merges make the origins of sets of
changesets pretty clear.
In addition, the merge commit allows you to review the aggregate of
these changesets in one convenient clump, if you choose.

> 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.

I'm really looking forward to seeing how evolution drives the review process.
However, I completely fail to see the point of "integrating" it with
email. Isn't the whole point of evolution that I can push changesets
somewhere, modify/reorder/etc. them nondestructively with history, and
then finalize them? Where does email fit into this process at all? If
anything, this is _even more_ suited to an interactive web interface.


Mercurial is of course not my project, and I'm far from being a
regular contributor, but I see this enforced workflow hurting the
project in three significant ways:
 * High barrier to entry: it is significantly more hassle to submit a
patch to mercurial than to any other project in my experience, with
the possible exception of the Linux kernel. This discourages
first-time and casual contributions.
 * Vote of no confidence: the fact that a modern DVCS project handles
internal contributions by emailing patches around doesn't project a
lot of confidence in the tool itself. There is no technical reason
that mercurial development could not actually be happening in CVS.
Would you feel good about an IDE whose developers mainly used vim?
 * Worms in the dogfood: I argue that core features of mercurial
suffer from bugs, incompleteness, and "papercut" workflow issues,
because the mercurial project itself doesn't use them. The two main
examples I can see are subrepos and branches.

Levi (Tak)