Repository: smarkets/marge-bot

Issue:
Don't assume a branch called `master` exists when removing the merged branch
It does not always merge request should be merged into `master`. E.g., in some git workflows, MR should be merged into `develop` branch. I propose to add configuration parameter `--branch-name-merge-into <string name>` for such situations.

Discussion:

Patch:
diff --git a/marge/git.py b/marge/git.py
index 29a7684..df8cfc5 100644
--- a/marge/git.py
+++ b/marge/git.py
@@ -107,9 +107,9 @@ class Repo(namedtuple('Repo', 'remote_url local_path ssh_key_file timeout')):
             raise
         return self.get_commit_hash()
 
-    def remove_branch(self, branch):
-        assert branch != 'master'
-        self.git('checkout', 'master', '--')
+    def remove_branch(self, branch, *, new_current_branch='master'):
+        assert branch != new_current_branch
+        self.git('checkout', new_current_branch, '--')
         self.git('branch', '-D', branch)
 
     def push_force(self, branch, source_repo_url=None):
diff --git a/marge/job.py b/marge/job.py
index 613a649..c7196c9 100644
--- a/marge/job.py
+++ b/marge/job.py
@@ -363,8 +363,8 @@ def update_from_target_branch_and_push(
         # A failure to clean up probably means something is fucked with the git repo
         # and likely explains any previous failure, so it will better to just
         # raise a GitError
-        if source_branch != 'master':
-            repo.remove_branch(source_branch)
+        if source_branch != target_branch:
+            repo.remove_branch(source_branch, new_current_branch=target_branch)
         else:
             assert source_repo_url is not None
 


Test patch:
diff --git a/tests/test_git.py b/tests/test_git.py
index 9ceaa1a..2ebc79c 100644
--- a/tests/test_git.py
+++ b/tests/test_git.py
@@ -112,6 +112,13 @@ class TestRepo(object):
         assert get_calls(mocked_run) == []
 
     def test_remove_branch(self, mocked_run):
+        self.repo.remove_branch('some_branch', new_current_branch='devel')
+        assert get_calls(mocked_run) == [
+            'git -C /tmp/local/path checkout devel --',
+            'git -C /tmp/local/path branch -D some_branch',
+        ]
+
+    def test_remove_branch_default(self, mocked_run):
         self.repo.remove_branch('some_branch')
         assert get_calls(mocked_run) == [
             'git -C /tmp/local/path checkout master --',
@@ -120,7 +127,7 @@ class TestRepo(object):
 
     def test_remove_master_branch_fails(self, unused_mocked_run):
         with pytest.raises(AssertionError):
-            self.repo.remove_branch('master')
+            self.repo.remove_branch('meister', new_current_branch='meister')
 
     def test_push_force(self, mocked_run):
         mocked_run.return_value = mocked_stdout(b'')