I'm having an issue where the action still merges a major release even if the target is set to minor.

[simoneb]

I'm having an issue where the action still merges a major release even if the target is set to minor.

For example, React v17.0.2 to v18.0.0 still gets merged.

    automerge:
        needs: analyze
        runs-on: ubuntu-latest
        permissions:
            pull-requests: write
            contents: write
        if: ${{ github.event_name == 'pull_request' }}
        steps:
            -   uses: fastify/github-action-merge-dependabot@v3
                with:
                    github-token: ${{ secrets.GITHUB_TOKEN }}
                    target: minor

Example PR: https://github.com/austins/smoothnanners-web/pull/17

Originally posted by @austins in #124 (comment)

[climba03003]

It's always true if the title do not match regexp.

github-action-merge-dependabot/src/checkTargetMatchToPR.js

Lines 12 to 16 in 116f1db

	const match = expression.exec(prTitle)
	if (!match) {
	return true
	}

[simoneb]

Ah I can see that that dependabot PR is doing something we're not expecting here! It's bumping both react and react-dom. I've never seen this case before, I guess dependabot is getting smarter. We need a different strategy here, all of the assumptions we made in the action rely on a single package being bumped

[simoneb]

@wilkmaia is looking into this

[wilkmaia]

@simoneb based on what I've seen, this is kind of an expected behavior in the code right now, even if not desired.

Specifically, looking at this test we can see it was actually expected that if the PR title didn't match the target match expression (/from ([^\s]+) to ([^\s]+)/) the target option would be ignored.

Moving forward we might want to improve that specific segment. Instead of simply checking the PR title we might want to check what's actually going on in the package.json file. Maybe matching the package name(s) in the PR title with the versions in package.json.

I'll work on that change for now but if you don't think that's not a good approach we can re-think the strategy.

---

Edit: only now I've seen the comments above lol.

[wilkmaia]

If this issue is ever implemented we might want to drop checking the PR title altogether, given the PR template they implement might involve setting the title as well.

[austins]

I believe there was also a case where it still approved a merge to a major release with a valid semver format, although the config was set to target minor (example PR). However, this was with fastify/[email protected].

For this case with React, yes, the title has two dependencies and dependabot isn't including the version numbers in the title. We could parse the PR message for Updates react from 17.0.2 to 18.0.0 and Updates react-dom from 17.0.2 to 18.0.0, however that may not be reliable if dependabot ever changes their template again. I would say that checking the diff in package.json would be the most consistent and reliable option.

Thanks Fastify team for looking into this!

[simoneb]

I agree that looking at the diff is probably the only reliable way.