remove determineMerge and set default value directly

googleapis · Sep 19, 2024 · eb0b59c · eb0b59c
1 parent 9092b4b
commit eb0b59c
Show file tree

Hide file tree

Showing 2 changed files with 11 additions and 95 deletions.
diff --git a/src/factories/plugin-factory.ts b/src/factories/plugin-factory.ts
@@ -18,7 +18,6 @@ import {
   RepositoryConfig,
   SentenceCasePluginConfig,
   GroupPriorityPluginConfig,
-  WorkspacePluginConfig,
 } from '../manifest';
 import {GitHub} from '../github';
 import {ManifestPlugin} from '../plugin';
@@ -56,6 +55,8 @@ export type PluginBuilder = (options: PluginFactoryOptions) => ManifestPlugin;
 
 const pluginFactories: Record<string, PluginBuilder> = {
   'linked-versions': options =>
+    // NOTE: linked-versions had already have a different behavior about merging
+    // see test/plugins/compatibility/linked-versions-workspace.ts
     new LinkedVersions(
       options.github,
       options.targetBranch,
@@ -65,7 +66,6 @@ const pluginFactories: Record<string, PluginBuilder> = {
       {
         ...options,
         ...(options.type as WorkspacePluginOptions),
-        merge: determineMerge(options),
       }
     ),
   'cargo-workspace': options =>
@@ -76,7 +76,9 @@ const pluginFactories: Record<string, PluginBuilder> = {
       {
         ...options,
         ...(options.type as WorkspacePluginOptions),
-        merge: determineMerge(options),
+        merge:
+          (options.type as WorkspacePluginOptions).merge ??
+          !options.separatePullRequests,
       }
     ),
   'node-workspace': options =>
@@ -87,7 +89,9 @@ const pluginFactories: Record<string, PluginBuilder> = {
       {
         ...options,
         ...(options.type as WorkspacePluginOptions),
-        merge: determineMerge(options),
+        merge:
+          (options.type as WorkspacePluginOptions).merge ??
+          !options.separatePullRequests,
       }
     ),
   'maven-workspace': options =>
@@ -98,7 +102,9 @@ const pluginFactories: Record<string, PluginBuilder> = {
       {
         ...options,
         ...(options.type as WorkspacePluginOptions),
-        merge: determineMerge(options),
+        merge:
+          (options.type as WorkspacePluginOptions).merge ??
+          !options.separatePullRequests,
       }
     ),
   'sentence-case': options =>
@@ -155,24 +161,3 @@ export function unregisterPlugin(name: string) {
 export function getPluginTypes(): readonly VersioningStrategyType[] {
   return Object.keys(pluginFactories).sort();
 }
-
-export function determineMerge(
-  options: PluginFactoryOptions
-): boolean | undefined {
-  // NOTE: linked-versions had already have a different behavior when this code wrote
-  // see test/plugins/compatibility/linked-versions-workspace.ts
-  if (typeof options.type === 'string' && options.type !== 'linked-versions') {
-    return !options.separatePullRequests;
-  }
-  if (typeof options.type !== 'string') {
-    const type = options.type as
-      | LinkedVersionPluginConfig
-      | WorkspacePluginConfig;
-    if (typeof type.merge === 'undefined' && type.type !== 'linked-versions') {
-      return !options.separatePullRequests;
-    }
-    return type.merge;
-  }
-  // return undefined due to relying on the default behavior of the plugin constructor
-  return undefined;
-}
diff --git a/test/factories/plugin-factory.ts b/test/factories/plugin-factory.ts
@@ -19,7 +19,6 @@ import {
   getPluginTypes,
   registerPlugin,
   unregisterPlugin,
-  determineMerge,
 } from '../../src/factories/plugin-factory';
 import {expect} from 'chai';
 import {LinkedVersions} from '../../src/plugins/linked-versions';
@@ -177,72 +176,4 @@ describe('PluginFactory', () => {
       expect(allTypes).to.contain(pluginType);
     });
   });
-  describe('determineMerge', () => {
-    const pluginOptions = {
-      github,
-      repositoryConfig: {},
-      targetBranch: 'main',
-      manifestPath: '.manifest.json',
-    };
-    it('should return false', () => {
-      const separatePullRequests = true;
-
-      expect(
-        determineMerge({
-          ...pluginOptions,
-          separatePullRequests,
-          type: 'node-workspace',
-        })
-      ).to.be.false;
-
-      expect(
-        determineMerge({
-          ...pluginOptions,
-          separatePullRequests,
-          type: {
-            type: 'maven-workspace',
-          },
-        })
-      ).to.be.false;
-    });
-    it('should return true', () => {
-      expect(
-        determineMerge({
-          ...pluginOptions,
-          separatePullRequests: false,
-          type: 'cargo-workspace',
-        })
-      ).to.be.true;
-
-      expect(
-        determineMerge({
-          ...pluginOptions,
-          separatePullRequests: true,
-          type: {
-            type: 'node-workspace',
-            merge: true,
-          },
-        })
-      ).to.be.true;
-    });
-    it('should return undefined', () => {
-      expect(
-        determineMerge({
-          ...pluginOptions,
-          separatePullRequests: true,
-          type: 'linked-versions',
-        })
-      ).to.be.undefined;
-
-      expect(
-        determineMerge({
-          ...pluginOptions,
-          separatePullRequests: true,
-          type: {
-            type: 'linked-versions',
-          },
-        })
-      ).to.be.undefined;
-    });
-  });
 });