fix: Merge commits should be versioned

[lohnn]

Description

Merge commits were not versioned. Example:

Merged PR 123: fix(3305): change url launcher to open externally

Fixes a regression of #258.

Me and @spydon fixed this together :)

Type of Change

• ✨ feat -- New feature (non-breaking change which adds functionality)
• 🛠️ fix -- Bug fix (non-breaking change which fixes an issue)
• ❌ ! -- Breaking change (fix or feature that would cause existing functionality to change)
• 🧹 refactor -- Code refactor
• ✅ ci -- Build configuration change
• 📝 docs -- Documentation
• 🗑️ chore -- Chore

[CLAassistant]

All committers have signed the CLA.

[blaugold]

I think there is a bit of inconsistency in the current code which we need to resolve to make this fully work.

In conventional_commit we are currently parsing merge commits as a ConventionalCommit even if it does not contain a correct header (fix: ...). Any commit that starts with Merge is allowed and gets some defaults.

melos/packages/conventional_commit/lib/conventional_commit.dart

Lines 74 to 84 in 9ea87a3

	if (headerMatch == null) {
	if (isMergeCommit) {
	return ConventionalCommit._(
	header: header,
	isMergeCommit: isMergeCommit,
	isBreakingChange: false,
	scopes: [],
	);
	}
	return null;
	}

This is not part of the Conventional Commit spec. I don't think we should do that and return null in that case, since it's not an actual Conventional Commit. We will need to know that every ConventionalCommit is an actual Conventional Commit for this PR.

There are also two places in the changelog generation that need to be updated. _filteredAndSortedCommits currently filters out all merge commits:

melos/packages/melos/lib/src/common/changelog.dart

Lines 213 to 214 in 9ea87a3

	!commit.parsedMessage.isMergeCommit &&
	commit.parsedMessage.isVersionableCommit,

And merge commits get special cased (even though they will always be filtered out before 🤷), which they shouldn't be with the changes in this PR:

melos/packages/melos/lib/src/common/changelog.dart

Lines 173 to 176 in 9ea87a3

	if (parsedMessage.isMergeCommit) {
	writePunctuated(processCommitHeader(parsedMessage.header));
	} else {
	writeBold(parsedMessage.type!.toUpperCase());

[lohnn]

Looking around, it seems a the way people found around this problem was to add a non-capturing group to the input to the header regex as input parameter. Such as this.
Would the proper way of solving this issue here be to add support for header regex override?

[blaugold]

I think directly supporting prefixes like Merged PR 123: is good, so users don't have to do extra set up.

My concern is that we distinguish Merged PR #24: fix: ... (which is a valid, versionable conventional commit), from Merged foo into main. Both parse as ConventionalCommits, but only the former commit has a type and that is what we test in isVersionableCommit, so that's fine.

I've added some tests and added the changes to fix changelog generation.

[lohnn]

Thanks a lot 🙏
Happy to see even more tests are added due to this PR as well.

Anything else you want me to do? Happy to help if there is.

[as19ish]

Can we add support for bitbucket ? In bitbucket also when we try to merge PR it add this message as default.
Merged in feature/DM-175-tag-management (pull request 156)

[lohnn]

That commit message does not follow conventional commits. It has to include at least a <type>:. in the commit message, else it won't be able to parse as a conventional commit.

[as19ish]

@lohnn yes we follow conventional commit but when we try to merge bitbucket add some text from there end.

[lohnn]

So basically what you would need is to skip a merge header alltogether?
Such as this https://github.com/conventional-changelog/conventional-changelog/tree/master/packages/conventional-commits-parser#merge ?

[blaugold]

@as19ish Could you open a new issue with details about how you merge commits (rebase, squash, merge), what bitbucket adds and how this breaks versioning?

Let's handle that separately, since this PR is about fixing a regression.

[spydon]

Thanks for merging @blaugold!
Any chance we could have a release including this? 😇
(Also, do you mind putting the hacktoberfest-accepted tag on the PR? I know Johannes is trying to finish the Invertase Hacktoberfest and only the ones with the tag count. 😄)

[blaugold]

I think we have a bunch of changes people are looking forward to. @Salakar Could you publish a new release?