summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorRémy Coutable <remy@rymai.me>2016-05-19 19:04:29 +0000
committerRémy Coutable <remy@rymai.me>2016-05-19 19:04:29 +0000
commite4a1af83532006f2b11a323cdabfcf8f042fa3c7 (patch)
tree6119de628487f23a61acd92e61dfdba9e39af388
parent0d5766e6f3840ce21ad56f8738299a85652cdd0b (diff)
parent37415e58f84fb3419eb26673e3eba909e1cb63c1 (diff)
downloadgitlab-ce-e4a1af83532006f2b11a323cdabfcf8f042fa3c7.tar.gz
Merge branch '12724-wrong-sorting-of-commit-order-in-mr-view' into 'master'
Resolve "Wrong sorting of commit order in MR view?" !4052 fixed this for the most obvious cases, but there were still some problems. Here's my test case: I have a branch where I was suffering from an unfortunate issue. Every other commit I made had its commit date set to one day before it should have been. (Perhaps my system clock was misbehaving.) ```shell for i in {1..10} do echo $i > $i git add $i GIT_COMMITTER_DATE=`date -v -$((i % 2))d` git commit -m $i done ``` The git CLI still gives me the commits in the right order, but I can see that the timestamps alternate between two values: ```shell $ git log --format='%h %ct %p %s' master...HEAD f0c3108 1463646313 3d38a13 10 3d38a13 1463559913 67f419b 9 67f419b 1463646313 74330c0 8 74330c0 1463559913 56361d7 7 56361d7 1463646313 ba1b60c 6 ba1b60c 1463559913 f91497d 5 f91497d 1463646313 79c5e57 4 79c5e57 1463559913 b953cef 3 b953cef 1463646313 12fc411 2 12fc411 1463559913 835715b 1 ``` Unfortunately, GitLab didn't like this _at all_. Here's what the commits on my MR from that branch looked like: ![image](/uploads/fdc38e932eeedcb77de9ec4b50ad1476/image.png) That's because we were sorting the commits by date, which is safe if they are in that order anyway. If they aren't, then because Ruby's sorting isn't stable, we lose even the ordering among the correctly-ordered commits with the same timestamp. After these changes (and reloading the MR's diff), this looks like: ![image](/uploads/b09fb0f51359c1c89484e713e859512b/image.png) The commits view was also wrong, but in a slightly different way. In table form: | View | Before | After | | --- | --- | --- | | Commit list | 10, 8, 6, 4, 2, 9, 7, 5, 3, 1 | 10, 9, 8, 7, 6, 5, 4, 3, 2, 1 | | MR commits | 10, 2, 8, 4, 6, 5, 7, 3, 9, 1 | 10, 9, 8, 7, 6, 5, 4, 3, 2, 1 | Closes #12724 See merge request !4208
-rw-r--r--CHANGELOG2
-rw-r--r--app/models/merge_request_diff.rb4
-rw-r--r--app/views/projects/commits/_commits.html.haml2
3 files changed, 3 insertions, 5 deletions
diff --git a/CHANGELOG b/CHANGELOG
index 9613332774f..422718dec9a 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -45,7 +45,7 @@ v 8.8.0 (unreleased)
- Add support for supressing text diffs using .gitattributes on the default branch (Matt Oakes)
- Add eager load paths to help prevent dependency load issues in Sidekiq workers. !3724
- Added multiple colors for labels in dropdowns when dups happen.
- - Always group commits by server timezone, not commit timestamp
+ - Show commits in the same order as `git log`
- Improve description for the Two-factor Authentication sign-in screen. (Connor Shea)
- API support for the 'since' and 'until' operators on commit requests (Paco Guzman)
- Fix Gravatar hint in user profile when Gravatar is disabled. !3988 (Artem Sidorenko)
diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb
index 6ad8fc3f034..7d5103748f5 100644
--- a/app/models/merge_request_diff.rb
+++ b/app/models/merge_request_diff.rb
@@ -98,9 +98,7 @@ class MergeRequestDiff < ActiveRecord::Base
commits = compare.commits
if commits.present?
- commits = Commit.decorate(commits, merge_request.source_project).
- sort_by(&:created_at).
- reverse
+ commits = Commit.decorate(commits, merge_request.source_project).reverse
end
commits
diff --git a/app/views/projects/commits/_commits.html.haml b/app/views/projects/commits/_commits.html.haml
index 82f39e59284..7283a78a64e 100644
--- a/app/views/projects/commits/_commits.html.haml
+++ b/app/views/projects/commits/_commits.html.haml
@@ -3,7 +3,7 @@
- commits, hidden = limited_commits(@commits)
-- commits.group_by { |c| c.committed_date.in_time_zone.to_date }.sort.reverse.each do |day, commits|
+- commits.chunk { |c| c.committed_date.in_time_zone.to_date }.each do |day, commits|
.row.commits-row
.col-md-2.hidden-xs.hidden-sm
%h5.commits-row-date