From b007bd810175dac80a509fb0c066d854f044fb96 Mon Sep 17 00:00:00 2001
From: Robert Speicher <rspeicher@gmail.com>
Date: Wed, 21 Sep 2016 17:22:54 +0300
Subject: [PATCH] Improve how MergeCommit#merge_commit_message builds the
message
Now a merge request with a blank description will no longer produce a
merge commit message like this:
```
Merge branch 'foo' into 'master'
Bring the wonders of foo into the world
See merge request !7283
```
What an improvement! :tada:
---
app/models/merge_request.rb | 12 +++++------
spec/models/merge_request_spec.rb | 36 +++++++++++++++++++++++++++++++
2 files changed, 41 insertions(+), 7 deletions(-)
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index 2dcf7f89bfc..aec555dcec0 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -590,13 +590,11 @@ class MergeRequest < ActiveRecord::Base
end
def merge_commit_message
- message = "Merge branch '#{source_branch}' into '#{target_branch}'"
- message << "\n\n"
- message << title.to_s
- message << "\n\n"
- message << description.to_s
- message << "\n\n"
- message << "See merge request !#{iid}"
+ message = "Merge branch '#{source_branch}' into '#{target_branch}'\n\n"
+ message << "#{title}\n\n"
+ message << "#{description}\n\n" if description.present?
+ message << "See merge request #{to_reference}"
+
message
end
diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb
index 12df6adde44..580a3235127 100644
--- a/spec/models/merge_request_spec.rb
+++ b/spec/models/merge_request_spec.rb
@@ -328,6 +328,42 @@ describe MergeRequest, models: true do
end
end
+ describe '#merge_commit_message' do
+ it 'includes merge information as the title' do
+ request = build(:merge_request, source_branch: 'source', target_branch: 'target')
+
+ expect(request.merge_commit_message)
+ .to match("Merge branch 'source' into 'target'\n\n")
+ end
+
+ it 'includes its title in the body' do
+ request = build(:merge_request, title: 'Remove all technical debt')
+
+ expect(request.merge_commit_message)
+ .to match("Remove all technical debt\n\n")
+ end
+
+ it 'includes its description in the body' do
+ request = build(:merge_request, description: 'By removing all code')
+
+ expect(request.merge_commit_message)
+ .to match("By removing all code\n\n")
+ end
+
+ it 'includes its reference in the body' do
+ request = build_stubbed(:merge_request)
+
+ expect(request.merge_commit_message)
+ .to match("See merge request #{request.to_reference}")
+ end
+
+ it 'excludes multiple linebreak runs when description is blank' do
+ request = build(:merge_request, title: 'Title', description: nil)
+
+ expect(request.merge_commit_message).not_to match("Title\n\n\n\n")
+ end
+ end
+
describe "#reset_merge_when_build_succeeds" do
let(:merge_if_green) do
create :merge_request, merge_when_build_succeeds: true, merge_user: create(:user),
--
GitLab