From 7aaa8e8db2c2cf3f7746ea5e28bca01e26d5c91d Mon Sep 17 00:00:00 2001
From: Marin Jankovski <maxlazio@gmail.com>
Date: Fri, 28 Nov 2014 11:29:11 +0100
Subject: [PATCH] Add comments around code to make it easier to understand,
 move remove_branch code into a separate method.

---
 lib/gitlab/satellite/merge_action.rb | 36 +++++++++++++++++++++++-----
 1 file changed, 30 insertions(+), 6 deletions(-)

diff --git a/lib/gitlab/satellite/merge_action.rb b/lib/gitlab/satellite/merge_action.rb
index 43dce0f0a5..76889377de 100644
--- a/lib/gitlab/satellite/merge_action.rb
+++ b/lib/gitlab/satellite/merge_action.rb
@@ -30,12 +30,14 @@ module Gitlab
         in_locked_and_timed_satellite do |merge_repo|
           prepare_satellite!(merge_repo)
 
+          # If rebase before merge
+          # Attempt a rebase and return if succesful.
+          # If unsuccesful, continue with regular merge.
           if merge_request.should_rebase
             if rebase_in_satellite!(merge_repo)
               merge_repo.git.push(default_options, :origin, merge_request.target_branch)
+              remove_source_branch(merge_repo)
               return true
-            else
-              prepare_satellite!(merge_repo)
             end
           end
 
@@ -45,10 +47,7 @@ module Gitlab
             merge_repo.git.push(default_options, :origin, merge_request.target_branch)
 
             # remove source branch
-            if merge_request.should_remove_source_branch && !project.root_ref?(merge_request.source_branch) && !merge_request.for_fork?
-              # will raise CommandFailed when push fails
-              merge_repo.git.push(default_options, :origin, ":#{merge_request.source_branch}")
-            end
+            remove_source_branch(merge_repo)
             # merge, push and branch removal successful
             true
           end
@@ -145,6 +144,21 @@ module Gitlab
         handle_exception(ex)
       end
 
+      # Rebases before merging the source_branch into the target_branch in the satellite.
+      # Before starting the rebase, clears out the satellite.
+      # Returns false if rebase cannot be done cleanly and resets the satellite.
+      # Returns true otherwise.
+
+      # Eg. of clean rebase
+      # source_branch: feature
+      # target_branch: master
+      #
+      # git checkout feature
+      # git merge master
+      # git rebase master
+      # git checkout master
+      # git merge feature
+      # git push remote master
       def rebase_in_satellite!(repo)
         update_satellite_source_and_target!(repo)
         repo.git.checkout(default_options({b: true}), merge_request.source_branch, "source/#{merge_request.source_branch}")
@@ -158,6 +172,7 @@ module Gitlab
         else
           repo.git.rebase(default_options, "--abort")
           Gitlab::AppLogger.info "Rebasing in in #{merge_request.source_project.path_with_namespace} MR!#{merge_request.id} aborted, rebase manually."
+          prepare_satellite!(repo)
           false
         end
       rescue Grit::Git::CommandFailed => ex
@@ -172,6 +187,15 @@ module Gitlab
       rescue Grit::Git::CommandFailed => ex
         handle_exception(ex)
       end
+
+      def remove_source_branch(repo)
+        if merge_request.should_remove_source_branch && !project.root_ref?(merge_request.source_branch) && !merge_request.for_fork?
+          # will raise CommandFailed when push fails
+          repo.git.push(default_options, :origin, ":#{merge_request.source_branch}")
+        end
+      rescue Grit::Git::CommandFailed => ex
+        handle_exception(ex)
+      end
     end
   end
 end