Sort commits topologically first instead of by author date

[cirosantilli]

Like git log --topo-order does by default. E.g.: https://github.com/cirosantilli/test-log-order/commits/master , screenshot 2015-05-01.

Generated with:

git init
touch '0'
git add '0'
date='2011-01-01 00:01:01 +0000'
GIT_COMMITTER_DATE="$date" git commit  -m '0' --date "$date"
touch '1'
git add '1'
date='2010-01-01 00:01:01 +0000'
GIT_COMMITTER_DATE="$date" git commit  -m '1' --date "$date"

Tree:

 0 --> 1

Actual log:

Commit	Date
0	2011
1	2010

Expected log:

Commit	Date
1	2010
0	2011

The actual tree structure is more important than the timestamp, which is just an arbitrary value that can be controlled by users.

For example, if an old commit gets merged later, I'd expect to see it on the top of the log as the merge date is what matters most.

Not to mention my evil desire to annoy projects by making a future max commit cirosantilli/test-git-web-interface@ff86a7b, create a fake account, find a typo on some famous project with aspell and make a pull request. I bet it would pass, and when the project admins notice it, they would likely be forced to force push it away. MUAHAHAHA. But I won't do it :-)

Maybe this was mentioned at: https://help.github.com/articles/why-are-my-commits-in-the-wrong-order/

IMHO the best option is --topo-order from man git-log, as it shows the most "logical" topo sort possible. libgit2 even has it already: https://libgit2.github.com/libgit2/ex/HEAD/log.html#section-23

[cirosantilli]

Standard from Ivan today.

[jmm]

I just noticed this in the list of commits on one of my PRs. They're not in the order I expected and I wondered if I screwed up my rebase, until I checked. Does this mean that GitHub said that's their intended behavior?:

Standard from Ivan today.

[cirosantilli]

@jmm hey, I cryptically meant that Ivan gave a standard reply that says nothing except acknowledge my email :-) So I don't know what they think about it.

[jmm]

@cirosantilli ah, gotcha.

[toejough]

from https://help.github.com/articles/why-are-my-commits-in-the-wrong-order/

If you rewrite your commit history via git rebase or a force push, you may notice that your commit sequence is out of order ...

So, there is acknowledgement that the chosen sort method presents commits in the wrong order.

GitHub emphasizes Pull Requests as a space for discussion. All aspects of it ... are represented in a chronological order.

It's not clear to me how only showing things in the wrong order (where the 'right' order is the way the committer ordered them during rebase) fosters better discussion. If anything, I've experienced the opposite - that it confuses both the pull request submitters and reviewers, leading to wasted time and brain cycles better spent on creating and reviewing actual content.

If you always want to see commits in order, we recommend not using git rebase.

The rebase command is really useful for cleaning up commit histories by reordering, squashing, splitting, and rewording commits, making them easier for reviewers to understand. Would it be better if everyone just committed the right code in the right order with the right organization and messages the first time? Absolutely, but that is a fantasy world. Rather than give up a tool that enables better commit organization, it seems more sensible to display commits in the committer's intended order.

[andersk]

Fixing this would not involve a huge paradigm shift. GitHub currently sorts the commits in a PR by author date (not sure if this was always the case, but I just checked). There is no reason they couldn’t simply use the commit date, rather than the author date, as the commit’s insertion point in the same chronological stream. Ties in the commit date ordering would be broken by another monotonic measure, such as the number of ancestor commits.

For people who follow GitHub’s current advice and never use git rebase or similar tools, the commit date will be equal to the author date and nothing will change.

For people who do use git rebase, the commit date is a much better indicator of what Git users expect “chronological” to mean. It’s when work on the commit was finished, rather than when it was started. When rewriting a commit, Git bumps its commit date to the current time; then, when replaying its descendants on top of it, Git bumps their commit dates as well. Because the replays may happen within 1 second, the resulting commit dates may be equal—hence the need for the tie breaker—but they will never go backwards (barring clock skew across machines or deliberate manual manipulation).

[timabbott]

+1, sorting by commit date, rather than author date, is definitely the correct solution here (I just came to this issue with the intent of posting what @andersk wrote).

[cirosantilli]

@timabbott note that I'm proposing topological sort, not commit date. If you want that, consider opening a separate issue.

[andersk]

@cirosantilli Your original proposal is unclear on a couple of points:

• “instead of by commit date”—but GitHub doesn’t sort by commit date, it sorts by author date, and that is the real problem that causes commits to be misordered.
• “Like git log does by default”—but the default for git log is reverse chronological order, not --topo-order. On any sufficiently complicated repository (I tried git.git itself), git log, git log --topo-order, git log --date-order, and git log --author-date-order produce four different orders. In fact, the default git log misorders a handful of commits with respect to their parents, while the other three do extra work to avoid that. But the default is otherwise closest to --date-order.
• If GitHub were to sort commits topologically, what would they do with pull request comments posted between swapped commits?

I’m only trying to refine your proposal with a plausible solution that correctly orders commits with respect to their parents in all realistic cases, while allowing an implementation consistent with GitHub’s requirements, which they explained to me as follows:

Date: Fri, 12 Apr 2013 15:00:15 -0700
From: Yossef Mendelssohn support@github.com (GitHub Staff)
To: andersk@mit.edu
Subject: Re: Commits shown misordered on pull request page

Anders,

We view a Pull Request as an ongoing discussion, and so the main view ("discussion" tab) shows all the activity in chronological order. This includes comments and references as well as the commits. With that in mind, we show the commits in that same order, whether or not that matches the graph order that Git has them stored in.

This isn't perfect, especially in cases like barnowl/barnowl#130 where there is no extra discussion. We're working to improve the experience, as we run into this same sort of confusion from time to time if we rebase or otherwise alter the commit history.

Yossef

To literally implement --topo-order GitHub would almost certainly have to drastically modify the pull request UI, e.g. by separating comments from commits. To implement my refined proposal they need only make a small non-disruptive tweak to the sorting function.

Keeping in mind that this is an unofficial issue tracker, you’re of course welcome to propose other solutions. Just remember that if GitHub ever looks at this issue, they’re only likely to change anything if it’s easy and it won’t break anything for other users.

[cirosantilli]

@andersk agreed on first two points.

I'm not sure if it would be that hard to implement topo order on PRs, GitHub could just use the server's timestamp + topo order to order the magic commit comments.

[andersk]

@cirosantilli We’re talking about pull request comments, not commit comments. Pull request comments are not related to the Git graph and do not have a “topo order”.

[toejough]

Ahh, thank you for posting that email, @andersk. So, the real problem here is that GH is conflating two different concepts - commit history & code review (PR comments/code comments).

It's reasonable and in some cases desirable for long-term readability and organization to re-order and re-write your commit history.

It's entirely unreasonable and confusing to re-order or re-write your code review history.

It doesn't make much sense to tie the ordering of something with un-editable history with something with editable history. I wonder why it was done that way... It's good to hear that github is aware of the conflict and is trying to design a better interface, though.