Feat: Add strategy selection to dependabot automerge workflow

[ingvaar]

Type of Change

• Bugfix
• Enhancement / new feature
• Refactoring
• Documentation

Description

This PR adds a strategy option to dependabot automerge workflow.
It allows to select the merge strategy. The possible options are:

• rebase
• merge
• squash (default)

Checklist

• Add relevant labels (for example type of change or patch/minor/major)
• Make sure not to introduce some mistakes
• Update documentation
• Review the Contributing Guideline and sign CLA
• Reference relevant issue(s) and close them after merging

[github-actions]

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

[ingvaar]

I have read the CLA Document and I hereby sign the CLA

[flaxel]

The PR looks already good but would be nice to fix the linting action. 🙌

[soemo]

thanks for merging my first suggestion so fast. I wasn't even able to comment on it :-)

But I want to discuss another point.
Do we really need the eval command.

Can we not refactor it in that way
pseudo code

          MERGE_OPTIONS=''
          if [ "${{ inputs.rebase }}" == 'true' ]; then
            MERGE_OPTIONS+=" --rebase"
          else
            MERGE_OPTIONS+=" --squash"
          fi
          if [ ${{ inputs.force }} == 'true' ]; then
            MERGE_OPTIONS+=" --admin"
          fi
          else
            MERGE_OPTIONS+=" --auto --merge"
          fi
          echo "Executing merge command with the options: '${MERGE_OPTIONS}'"
          gh pr merge "$PR_URL" "${MERGE_OPTIONS}"

[ingvaar]

@soemo I really liked the concise approach you proposed 🙁

We don't need the eval here, and the pseudo code you proposed is nice.

But there is a concern here with the merge method:

if [ "${{ inputs.rebase }}" == 'true' ]; then
  MERGE_OPTIONS+=" --rebase"
else
  MERGE_OPTIONS+=" --squash"
fi

if [ "${{ inputs.force }}" == 'true' ]; then
  MERGE_OPTIONS+=" --admin"
else
  MERGE_OPTIONS+=" --auto --merge"
fi

With rebase as false and force as false, we will cumulate --squash and --merge, which are two different strategies.

Should I change the input to select the strategy instead of a boolean between rebase and merge/squash?

[soemo]

Should I change the input to select the strategy instead of a boolean between rebase and merge/squash?

That is a nice idea 👍 to have a strategy as input. If we set squash as default, the new code will be get much easier

[flaxel]

Really nice improvement! 🎉

[timdittler]

LGTM, but it would be great to test it in action before merging

my hints are included now

[wmentzel]

Hey @ingvaar !

The action ran into a problem in the lambda-edge-secure-media repo:

https://github.com/Staffbase/lambda-edge-secure-media/actions/runs/7867783102/job/21467013254?pr=91

[soemo]

How we move on with this PR?

[wmentzel]

How we move on with this PR?

We need to test it after the last change.

[wmentzel]

We need to test it after the last change.

We will test it in one of our repos with the next minor bump of a dependency. /cc @ingvaar

[wmentzel]

We again have to wait for suitable dependabot PR matching the condition: https://github.com/Staffbase/gha-workflows/blob/main/.github/workflows/template_automerge_dependabot.yml#L39

We'll run dependabot over the week to hopefully soon have some.

[wmentzel]

Good news: Finally, there was a dependbat PR to test this.
Bad news: It's not working. 😢

https://github.com/Staffbase/media-analytics/actions/runs/8223143665/job/22485477812?pr=87

Edit: I am on it!

[wmentzel]

The most recent version (HEAD of this brach => d43d6f6) of the action is not used in https://github.com/Staffbase/media-analytics/pull/87. Most likely due to caching...

[wmentzel]

Here a successful run: https://github.com/Staffbase/media-analytics/actions/runs/8224375573/job/22488121540?pr=87

This is the dependabot PR for testing: https://github.com/Staffbase/media-analytics/pull/87

@flaxel I think the force option never worked before 😅 : ff8cf9e

[flaxel]

@flaxel I think the force option never worked before 😅 : ff8cf9e

@wmentzel Why do you think that the action doesn't work? We are using it f.e. in the EXS and there we have no problems. 😄

[wmentzel]

@flaxel I think the force option never worked before 😅 : ff8cf9e

@wmentzel Why do you think that the action doesn't work? We are using it f.e. in the EXS and there we have no problems. 😄

I clarified this in a call with Falk. My assumption was based on a change which was introduced in this PR before I took over 😅. So, on main the action was never broken.