fix: Merge commits should be versioned (#407)

Co-authored-by: Gabriel Terwesten <gabriel@terwesten.net>
invertase · Oct 21, 2022 · 01d4cd0 · 01d4cd0
1 parent bb6563a
commit 01d4cd0
Show file tree

Hide file tree

Showing 5 changed files with 76 additions and 23 deletions.
diff --git a/packages/conventional_commit/test/conventional_commit_test.dart b/packages/conventional_commit/test/conventional_commit_test.dart
@@ -93,14 +93,25 @@ void main() {
       'correctly parses commit with prefix before conventional commit type',
       () {
         const commitMessage = 'Merged PR 404: feat(scope): new feature';
-        final conventionalCommit = ConventionalCommit.tryParse(commitMessage);
-        expect(conventionalCommit?.type, 'feat');
-        expect(conventionalCommit?.scopes, ['scope']);
-        expect(conventionalCommit?.description, 'new feature');
-        expect(conventionalCommit?.isMergeCommit, true);
+        final conventionalCommit = ConventionalCommit.tryParse(commitMessage)!;
+        expect(conventionalCommit.type, 'feat');
+        expect(conventionalCommit.scopes, ['scope']);
+        expect(conventionalCommit.description, 'new feature');
+        expect(conventionalCommit.isMergeCommit, true);
       },
     );
 
+    test('parses merge commits which are not conventional commits', () {
+      const commitMessage = 'Merged foo into bar';
+      final conventionalCommit = ConventionalCommit.tryParse(commitMessage)!;
+      expect(conventionalCommit.type, isNull);
+      expect(conventionalCommit.scopes, isEmpty);
+      expect(conventionalCommit.description, isNull);
+      expect(conventionalCommit.body, isNull);
+      expect(conventionalCommit.header, commitMessage);
+      expect(conventionalCommit.isMergeCommit, true);
+    });
+
     test('correctly handles messages with a `*` scope', () {
       final commit = ConventionalCommit.tryParse(commitMessageStarScope);
       expect(commit, isNotNull);
@@ -328,6 +339,10 @@ void main() {
             .isMergeCommit,
         isTrue,
       );
+      expect(
+        ConventionalCommit.tryParse('Merged PR #0: fix: foo')!.isMergeCommit,
+        isTrue,
+      );
       expect(
         ConventionalCommit.tryParse('docs!: foo bar')!.isMergeCommit,
         isFalse,

diff --git a/packages/melos/lib/src/common/changelog.dart b/packages/melos/lib/src/common/changelog.dart
@@ -170,20 +170,16 @@ extension ChangelogStringBufferExtension on StringBuffer {
           write(' ');
         }
 
-        if (parsedMessage.isMergeCommit) {
-          writePunctuated(processCommitHeader(parsedMessage.header));
-        } else {
-          writeBold(parsedMessage.type!.toUpperCase());
-          if (config.commands.version.includeScopes) {
-            if (parsedMessage.scopes.isNotEmpty) {
-              write('(');
-              write(parsedMessage.scopes.join(','));
-              write(')');
-            }
+        writeBold(parsedMessage.type!.toUpperCase());
+        if (config.commands.version.includeScopes) {
+          if (parsedMessage.scopes.isNotEmpty) {
+            write('(');
+            write(parsedMessage.scopes.join(','));
+            write(')');
           }
-          write(': ');
-          writePunctuated(processCommitHeader(parsedMessage.description!));
         }
+        write(': ');
+        writePunctuated(processCommitHeader(parsedMessage.description!));
 
         if (linkToCommits || includeCommitId) {
           final shortCommitId = commit.id.substring(0, 8);
@@ -208,11 +204,7 @@ List<RichGitCommit> _filteredAndSortedCommits(
   MelosPendingPackageUpdate update,
 ) {
   final commits = update.commits
-      .where(
-        (commit) =>
-            !commit.parsedMessage.isMergeCommit &&
-            commit.parsedMessage.isVersionableCommit,
-      )
+      .where((commit) => commit.parsedMessage.isVersionableCommit)
       .toList();
 
   // Sort so that Breaking Changes appear at the top.

diff --git a/packages/melos/lib/src/common/versioning.dart b/packages/melos/lib/src/common/versioning.dart
@@ -34,7 +34,6 @@ extension ConventionalCommitVersioningExtension on ConventionalCommit {
   /// Whether this commit should trigger a version bump in it's residing
   /// package.
   bool get isVersionableCommit {
-    if (isMergeCommit) return false;
     return isBreakingChange ||
         [
           'docs',

diff --git a/packages/melos/test/changelog_test.dart b/packages/melos/test/changelog_test.dart
@@ -59,6 +59,34 @@ void main() {
         contains('**FEAT**(a,b): c.'),
       );
     });
+
+    test('merge commit without conventional commit', () {
+      final workspace = buildWorkspaceWithRepository();
+      final package = workspace.allPackages['test_pkg']!;
+
+      expect(
+        renderCommitPackageUpdate(
+          workspace,
+          package,
+          testCommit(message: 'Merge foo into bar'),
+        ),
+        '## 0.0.0+1\n\n',
+      );
+    });
+
+    test('merge commit with conventional commit', () {
+      final workspace = buildWorkspaceWithRepository();
+      final package = workspace.allPackages['test_pkg']!;
+
+      expect(
+        renderCommitPackageUpdate(
+          workspace,
+          package,
+          testCommit(message: 'Merge PR #1: feat: a'),
+        ),
+        contains('**FEAT**: a.'),
+      );
+    });
   });
 
   group('linkToCommits', () {

diff --git a/packages/melos/test/common/versioning_test.dart b/packages/melos/test/common/versioning_test.dart
@@ -31,6 +31,25 @@ void main() {
           .isVersionableCommit,
       isFalse,
     );
+    expect(
+      ConventionalCommit.tryParse('Merged PR 1337: bar foo')!
+          .isVersionableCommit,
+      isFalse,
+    );
+    expect(
+      ConventionalCommit.tryParse('Merged PR 1337: fix(1338): bar foo')!
+          .isVersionableCommit,
+      isTrue,
+    );
+    expect(
+      ConventionalCommit.tryParse('Merged PR: fix(*): bar foo')!
+          .isVersionableCommit,
+      isTrue,
+    );
+    expect(
+      ConventionalCommit.tryParse('Merged foo into bar')!.isVersionableCommit,
+      isFalse,
+    );
   });
 
   test('semverReleaseType', () {