On 04/26/2015 08:50 PM, Thomas De Schampheleire wrote:
On Sun, Apr 26, 2015 at 7:56 PM, Mads Kiilerich <[email protected]> wrote:
On 04/25/2015 10:23 PM, Thomas De Schampheleire wrote:
# HG changeset patch
# User Thomas De Schampheleire <[email protected]>
# 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
_______________________________________________
kallithea-general mailing list
[email protected]
http://lists.sfconservancy.org/mailman/listinfo/kallithea-general

Reply via email to