diff --git a/runbot_merge/controllers.py b/runbot_merge/controllers.py index bb6c8709..2835742c 100644 --- a/runbot_merge/controllers.py +++ b/runbot_merge/controllers.py @@ -142,8 +142,8 @@ def handle_pr(event): pr_obj.staging_id, ) - # TODO: should we update squash as well? What of explicit squash commands? pr_obj.head = pr['head']['sha'] + pr_obj.squash = pr['commits'] == 1 return 'Updated {} to {}'.format(pr_obj.id, pr_obj.head) # don't marked merged PRs as closed (!!!) diff --git a/runbot_merge/models/pull_requests.py b/runbot_merge/models/pull_requests.py index 57843e0c..920de9cd 100644 --- a/runbot_merge/models/pull_requests.py +++ b/runbot_merge/models/pull_requests.py @@ -833,8 +833,6 @@ class Batch(models.Model): msg = pr.message author=None if pr.squash: - # FIXME: maybe should be message of the *first* commit of the branch? - # TODO: or depend on # of commits in PR instead of squash flag? commit = gh.commit(pr.head) msg = commit['message'] author = commit['author'] diff --git a/runbot_merge/tests/test_basic.py b/runbot_merge/tests/test_basic.py index 241cdb65..c95ebf41 100644 --- a/runbot_merge/tests/test_basic.py +++ b/runbot_merge/tests/test_basic.py @@ -536,6 +536,47 @@ class TestSquashing(object): ]).state == 'merged' assert prx.state == 'closed' + def test_pr_update_unsquash(self, repo, env): + """ + If a PR starts with 1 commit and a second commit is added, the PR + should be unflagged as squash + """ + m = repo.make_commit(None, 'initial', None, tree={'m': 'm'}) + m2 = repo.make_commit(m, 'second', None, tree={'m': 'm', 'm2': 'm2'}) + repo.make_ref('heads/master', m2) + + c1 = repo.make_commit(m, 'first', None, tree={'m': 'c1'}) + prx = repo.make_pr('title', 'body', target='master', ctid=c1, user='user') + pr = env['runbot_merge.pull_requests'].search([ + ('repository.name', '=', 'odoo/odoo'), + ('number', '=', prx.number), + ]) + assert pr.squash, "a PR with a single commit should be squashed" + + prx.push(repo.make_commit(c1, 'second2', None, tree={'m': 'c2'})) + assert not pr.squash, "a PR with a single commit should not be squashed" + + def test_pr_reset_squash(self, repo, env): + """ + If a PR starts at >1 commits and is reset back to 1, the PR should be + re-flagged as squash + """ + m = repo.make_commit(None, 'initial', None, tree={'m': 'm'}) + m2 = repo.make_commit(m, 'second', None, tree={'m': 'm', 'm2': 'm2'}) + repo.make_ref('heads/master', m2) + + c1 = repo.make_commit(m, 'first', None, tree={'m': 'c1'}) + c2 = repo.make_commit(c1, 'second2', None, tree={'m': 'c2'}) + prx = repo.make_pr('title', 'body', target='master', ctid=c2, user='user') + pr = env['runbot_merge.pull_requests'].search([ + ('repository.name', '=', 'odoo/odoo'), + ('number', '=', prx.number), + ]) + assert not pr.squash, "a PR with a single commit should not be squashed" + + prx.push(repo.make_commit(m, 'fixup', None, tree={'m': 'c2'})) + assert pr.squash, "a PR with a single commit should be squashed" + @pytest.mark.xfail(reason="removed support for squash+ command") def test_force_squash_merge(self, repo, env): m = repo.make_commit(None, 'initial', None, tree={'m': 'm'})