Commit b48c4b26 authored by Dmitriy Zaporozhets's avatar Dmitriy Zaporozhets

Refactor straight compare diff code

Signed-off-by: default avatarDmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
parent 7421c08a
...@@ -11,7 +11,7 @@ class Compare ...@@ -11,7 +11,7 @@ class Compare
end end
end end
def initialize(compare, project, straight = false) def initialize(compare, project, straight: false)
@compare = compare @compare = compare
@project = project @project = project
@straight = straight @straight = straight
...@@ -37,8 +37,6 @@ class Compare ...@@ -37,8 +37,6 @@ class Compare
alias_method :commit, :head_commit alias_method :commit, :head_commit
def base_commit def base_commit
return start_commit if @straight
return @base_commit if defined?(@base_commit) return @base_commit if defined?(@base_commit)
@base_commit = if start_commit && head_commit @base_commit = if start_commit && head_commit
...@@ -48,6 +46,18 @@ class Compare ...@@ -48,6 +46,18 @@ class Compare
end end
end end
def start_commit_sha
start_commit.try(:sha)
end
def base_commit_sha
base_commit.try(:sha)
end
def head_commit_sha
commit.try(:sha)
end
def raw_diffs(*args) def raw_diffs(*args)
@compare.diffs(*args) @compare.diffs(*args)
end end
...@@ -61,9 +71,9 @@ class Compare ...@@ -61,9 +71,9 @@ class Compare
def diff_refs def diff_refs
Gitlab::Diff::DiffRefs.new( Gitlab::Diff::DiffRefs.new(
base_sha: base_commit.try(:sha), base_sha: @straight ? start_commit_sha : base_commit_sha,
start_sha: start_commit.try(:sha), start_sha: start_commit_sha,
head_sha: commit.try(:sha) head_sha: head_commit_sha
) )
end end
end end
...@@ -167,11 +167,11 @@ class MergeRequestDiff < ActiveRecord::Base ...@@ -167,11 +167,11 @@ class MergeRequestDiff < ActiveRecord::Base
self == merge_request.merge_request_diff self == merge_request.merge_request_diff
end end
def compare_with(sha, straight = true) def compare_with(sha, straight: true)
# When compare merge request versions we want diff A..B instead of A...B # When compare merge request versions we want diff A..B instead of A...B
# so we handle cases when user does squash and rebase of the commits between versions. # so we handle cases when user does squash and rebase of the commits between versions.
# For this reason we set straight to true by default. # For this reason we set straight to true by default.
CompareService.new.execute(project, head_commit_sha, project, sha, straight) CompareService.new.execute(project, head_commit_sha, project, sha, straight: straight)
end end
private private
......
...@@ -3,7 +3,7 @@ require 'securerandom' ...@@ -3,7 +3,7 @@ require 'securerandom'
# Compare 2 branches for one repo or between repositories # Compare 2 branches for one repo or between repositories
# and return Gitlab::Git::Compare object that responds to commits and diffs # and return Gitlab::Git::Compare object that responds to commits and diffs
class CompareService class CompareService
def execute(source_project, source_branch, target_project, target_branch, straight = false) def execute(source_project, source_branch, target_project, target_branch, straight: false)
source_commit = source_project.commit(source_branch) source_commit = source_project.commit(source_branch)
return unless source_commit return unless source_commit
...@@ -27,6 +27,6 @@ class CompareService ...@@ -27,6 +27,6 @@ class CompareService
straight straight
) )
Compare.new(raw_compare, target_project, straight) Compare.new(raw_compare, target_project, straight: straight)
end end
end end
...@@ -7,13 +7,13 @@ describe CompareService, services: true do ...@@ -7,13 +7,13 @@ describe CompareService, services: true do
describe '#execute' do describe '#execute' do
context 'compare with base, like feature...fix' do context 'compare with base, like feature...fix' do
subject { service.execute(project, 'feature', project, 'fix', false) } subject { service.execute(project, 'feature', project, 'fix', straight: false) }
it { expect(subject.diffs.size).to eq(1) } it { expect(subject.diffs.size).to eq(1) }
end end
context 'straight compare, like feature..fix' do context 'straight compare, like feature..fix' do
subject { service.execute(project, 'feature', project, 'fix', true) } subject { service.execute(project, 'feature', project, 'fix', straight: true) }
it { expect(subject.diffs.size).to eq(3) } it { expect(subject.diffs.size).to eq(3) }
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment