[PATCH RFC] pullrequest: reverse order of changeset list

[Mads Kiilerich]

On 04/26/2015 08:50 PM, Thomas De Schampheleire wrote:
> On Sun, Apr 26, 2015 at 7:56 PM, Mads Kiilerich <mads at kiilerich.com> wrote:
>> On 04/25/2015 10:23 PM, Thomas De Schampheleire wrote:
>>> # HG changeset patch
>>> # User Thomas De Schampheleire <thomas.de.schampheleire at gmail.com>
>>> # Date 1429991014 -7200
>>> #      Sat Apr 25 21:43:34 2015 +0200
>>> # Node ID 7f62f8ef3acc8ce197156d138936a56329f0d151
>>> # Parent  471b0adcad4cb7e40db3599340fe1b6242d3a7b8
>>> pullrequest: reverse order of changeset list
>>
>> I see your point.
>>
>> But
>> * It is (or rather: was and would be) confusing when lists of changesets are
>> in different order in different places. Currently we avoid that by always
>> showing in the same order (and showing the graph to make it more clear how
>> they relate.)
>> * The changeset list is not necessarily linear - showing the graph helps
>> understanding how the changeset relates. "Nobody" would show a graph with
>> the most recent changes at the bottom. Showing a graph thus shows what the
>> order of the changesets is ... and do that we can't reverse the list.
>>
>>> When reviewing a pullrequest, it makes sense to start with the parent-most
>>> commit and end with the child-most commit (old to new).
>>
>> Perhaps. But it could be seen as: A pullrequest is building on top of a
>> (presumably) stable foundation. On top of that you add the changesets on top
>> of each other. So when reviewing something (or building a brick house), you
>> have to start from the bottom with the lowest revisions (or bricks).
>>
>>
>> My conclusion: I do not think it is a good idea. I think it is better to
>> teach people that they should do changeset reviews bottom-up. It can perhaps
>> be added if we get a mode where we ensure that a "PR" is linear (perhaps my
>> making it more like a series of patches like Phabricator and ReviewBoard).
> Previously, the 'order' of the displayed changesets could be derived
> from the logical revision number, but this has been removed.

It is just no longer enabled by default. You can enable 
show_revision_number if you want it. These numbers are however a source 
of error since they are "local" and different for server and client (at 
least for Mercurial) and thus can't be used. Also, when the revision 
numbers get high you can no longer easily at a glance see which one is 
bigger.

> So, I think we need another method now to clarify the order.

Even when it always is in the same order? Or do you assume that we need 
to be able to show lists in both orders? If so, I think I disagree on 
the premise.

The dots/circles for all changesets could perhaps be made something that 
looks more like a funnel where the lines from the previous changesets 
goes into the funnel. Or something like a box where it is clear that the 
ancestors goes into it while it is the whole thing that is used as an 
ancestor for other changesets.

It would probably be simpler to just put a number on all the changesets 
in the series. The number will be local for the PR but it will be small 
and the ordering will be obvious (even though the number itself will be 
unstable in the case where the topological ordering on the server 
somehow should change). I think that would be upstreamable. I still 
think it is a bad idea to show it top-down.

/Mads