futurelearn / squiddy

Github Actions bot
1 stars 1 forks source link

Teach squiddy how to do a bubble merge #3

Closed elenatanasoiu closed 4 years ago

elenatanasoiu commented 4 years ago

When does it trigger?

When you comment @squiddy-bot merge on your PR.

What does it do?

Rebases & merges your PR into master via bubble merge so that your git commit history is linear. This will allow us to lock the master branch and only allow merges via squiddy-bot.

Steps:

  1. Rebase your branch off master. This will create a merge commit which will be removed in the next step.
  2. Merge your branch into master without a merge commit. This will close the PR.
  3. Delete your branch.

There is also a README explaining how to add the action to a repo.

Example 1

PR: https://github.com/futurelearn/test-github-actions/pull/16

This PR introduced two new commits: “Update readme #1” and “Update readme #2" which were merged into master.

Screenshot 2020-07-24 at 17 45 51

The branch Squiddy merged was behind on master by one commit (“Add names to all our actions”). Squiddy did a rebase to add that commit to our branch and then did a merge back into master. Notice there is no resulting merge commit on master.

Finally it then deleted the branch because they’re a polite squid.

Screenshot 2020-07-24 at 18 02 27

NB: If the PR has conflicts with the base branch, these will need to be handled manually (e.g. if you've changed a file that has also been changed on master after you opened your PR).

Example 2 - Auto merging was not possible

https://github.com/futurelearn/test-github-actions/pull/13

In this case there was a conflict with the master branch. Squiddy will let you know an auto rebase and merge isn't possible.

Screenshot 2020-07-24 at 18 12 30

Feedback

  1. Are we ok with squiddy creating a merge commit on the PR as long as it removes it on master?
surminus commented 4 years ago

The one thing I'm confused about is removing the merge commit. I thought we explicitly wanted the merge commit, so why are we removing it?

marcotranchino commented 4 years ago

The one thing I'm confused about is removing the merge commit. I thought we explicitly wanted the merge commit, so why are we removing it?

I think you are right. I have made a couple of tests and verified that, currently, Squiddy merges without the bubble. As in the picture below (right side of the picture).

changes needed
marcotranchino commented 4 years ago

The clean merge bubble process is working with a combination of existing GitHub actions and with the Bubble Merge code as in this branch.

The bubble_merge.yml file in the workflow needs to be configured as follows:

name: Bubble Merge
on:
  issue_comment:
    types: [created]
    branches-ignore:
      - master

jobs:
  bubble_merge:
    name: Bubble Merge
    if: github.event.issue.pull_request != '' && contains(github.event.comment.body, '@squiddy-bot merge')
    runs-on: ubuntu-latest
    steps:
      - name: Checkout the latest code
        uses: actions/checkout@main
        with:
          fetch-depth: 0
      - name: Automatic Rebase
        uses: cirrus-actions/rebase@1.3.1
        env:
          GITHUB_TOKEN: ${{ secrets.SQUIDDY_BOT_GITHUB_TOKEN }}
      - name: Acknowledge Failure
        if: ${{ failure() }}
        uses: actions/github-script@0.9.0
        with:
          github-token: ${{secrets.SQUIDDY_BOT_GITHUB_TOKEN}}
          script: |
            await github.issues.createComment({
              issue_number: context.issue.number,
              owner: context.repo.owner,
              repo: context.repo.repo,
              body: 'Rebase was not possible. Please rebase manually and try again.'
            })
      - uses: futurelearn/squiddy/actions/bubble_merge@mt-et-bubble-merge
        env:
          GITHUB_TOKEN: ${{ secrets.SQUIDDY_BOT_GITHUB_TOKEN }}
          GITHUB_EVENT: ${{ toJson(github.event) }}

The acknowledge failure action is needed to comment on the PR if the rebase fails. There is no need to acknowledge success of the rebase as the squiddy/actions/bubble_merge action takes over and is responsible for further comments on the PR.

marcotranchino commented 4 years ago

When rebasing is not possible:

rebase not possible

After rebase and merge:

rebased and merge

surminus commented 4 years ago

If this works then I'm happy to merge it in! We can always iterate on improvements (particularly on speed of feedback on comments). Will be great to see it working out in the wild.

May need to update the README.

marcotranchino commented 4 years ago

If this works then I'm happy to merge it in! We can always iterate on improvements (particularly on speed of feedback on comments). Will be great to see it working out in the wild.

May need to update the README.

README updated 👍 :)