[PATCH RFC] pullrequest: reverse order of changeset list

[Thomas De Schampheleire]

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. So, I
think we need another method now to clarify the order. This could
either be by explicitly mentioning it in text (for example as I did in
this patch, feel free to suggest different wording), or perhaps by
adding some kind of small arrow in the graph? Of course, one would
still need to define what an arrow means, either descendant-of or
ancestor-of. For a graph with a -> b, I would assume that a is the
parent of b.
Note that adding the arrow is probably not so obvious as one would
need to extend the BranchRenderer javascript.

Best regards,
Thomas