[PATCH RFC] pullrequest: reverse order of changeset list

[Thomas De Schampheleire]

On Sun, Apr 26, 2015 at 9:15 PM, Mads Kiilerich <mads at kiilerich.com> wrote:
> 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.

I don't think we need to be able to show lists in both orders. But I
do think that we should not assume that users remember which order
Kallithea uses, because there are other tools (like Bitbucket) that
show pullrequest commits in the other order. Clarifying in some way
what the order is, is sufficient to handle this.

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

I like this idea: it's indeed simple, both to implement and to understand.

No worries, I'll keep the existing order ;-)
But to be clear: what do you consider number 1: the parent-most commit
(the one at the bottom), right?

Thanks,
Thomas