Revert "feat(ng-dev): add conditional autosquash merge strategy"

This reverts commit 95499f4.

Author: alan-agius4

.ng-dev/pull-request.mts

@@ -2,11 +2,7 @@ import {PullRequestConfig} from '../ng-dev/pr/config/index.js';
 
 /** Configuration for interacting with pull requests in the repo. */
 export const pullRequest: PullRequestConfig = {
-  githubApiMerge: {
-    default: 'rebase',
-    labels: [{pattern: 'merge: squash commits', method: 'squash'}],
-  },
-  conditionalAutosquashMerge: true,
+  githubApiMerge: false,
   requiredStatuses: [{name: 'test', type: 'check'}],
 
   // Disable target labeling in the dev-infra repo as we don't have

ng-dev/pr/config/index.ts

@@ -23,7 +23,7 @@ export interface GithubApiMergeStrategyConfig {
 }
 
 /** Configuration for the merge script. */
-export type PullRequestConfig = {
+export interface PullRequestConfig {
   /**
    * Configuration for the upstream remote. All of these options are optional as
    * defaults are provided by the common dev-infra github configuration.
@@ -63,25 +63,7 @@ export type PullRequestConfig = {
    * follow the canonical branching/versioning.
    */
   __noTargetLabeling?: boolean;
-} & {
-  /**
-   * Whether pull requests should be merged using a conditional autosquash strategy.
-   * If a pull request contains fixup or squash commits, the autosquash strategy
-   * will be used. Otherwise, the Github API merge strategy will be used.
-   */
-  conditionalAutosquashMerge?: boolean;
-
-  /**
-   * The configuration for merging pull requests using the Github API.
-   *
-   * This strategy is used as a fallback for the `conditionalAutosquashMerge` strategy,
-   * when a pull request does not contain any fixup or squash commits.
-   *
-   * This can be enabled if projects want to have their pull requests show up as
-   * `Merged` in the Github UI.
-   */
-  githubApiMerge: GithubApiMergeStrategyConfig;
-};
+}
 
 /** Loads and validates the merge configuration. */
 export function assertValidPullRequestConfig<T extends NgDevConfig>(
@@ -94,18 +76,10 @@ export function assertValidPullRequestConfig<T extends NgDevConfig>(
     );
   }
 
-  const {conditionalAutosquashMerge, githubApiMerge} = config.pullRequest;
-  if (githubApiMerge === undefined) {
+  if (config.pullRequest.githubApiMerge === undefined) {
     errors.push('No explicit choice of merge strategy. Please set `githubApiMerge`.');
   }
 
-  if (conditionalAutosquashMerge && !githubApiMerge) {
-    errors.push(
-      '`conditionalAutosquashMerge` requires a GitHub API merge strategy to inspect commit history. ' +
-        'Please configure `githubApiMerge` or disable `conditionalAutosquashMerge`.',
-    );
-  }
-
   if (errors.length) {
     throw new ConfigValidationError('Invalid `pullRequest` configuration', errors);
   }

ng-dev/pr/merge/merge-tool.ts

@@ -19,7 +19,6 @@ import {
 import {loadAndValidatePullRequest, PullRequest} from './pull-request.js';
 import {GithubApiMergeStrategy} from './strategies/api-merge.js';
 import {AutosquashMergeStrategy} from './strategies/autosquash-merge.js';
-import {ConditionalAutosquashMergeStrategy} from './strategies/conditional-autosquash-merge.js';
 import {GithubConfig, NgDevConfig} from '../../utils/config.js';
 import {assertValidReleaseConfig} from '../../release/config/index.js';
 import {
@@ -140,20 +139,9 @@ export class MergeTool {
       throw new UserAbortedMergeToolError();
     }
 
-    let strategy:
-      | AutosquashMergeStrategy
-      | ConditionalAutosquashMergeStrategy
-      | GithubApiMergeStrategy;
-
-    const {conditionalAutosquashMerge, githubApiMerge} = this.config.pullRequest;
-
-    if (conditionalAutosquashMerge) {
-      strategy = new ConditionalAutosquashMergeStrategy(this.git, githubApiMerge);
-    } else if (githubApiMerge) {
-      strategy = new GithubApiMergeStrategy(this.git, githubApiMerge);
-    } else {
-      strategy = new AutosquashMergeStrategy(this.git);
-    }
+    const strategy = this.config.pullRequest.githubApiMerge
+      ? new GithubApiMergeStrategy(this.git, this.config.pullRequest.githubApiMerge)
+      : new AutosquashMergeStrategy(this.git);
 
     // Branch or revision that is currently checked out so that we can switch back to
     // it once the pull request has been merged.

ng-dev/pr/merge/strategies/api-merge.ts

@@ -8,6 +8,7 @@
 
 import {RestEndpointMethodTypes} from '@octokit/plugin-rest-endpoint-methods';
 
+import {parseCommitMessage} from '../../../commit-message/parse.js';
 import {AuthenticatedGitClient} from '../../../utils/git/authenticated-git-client.js';
 import {GithubApiMergeMethod, GithubApiMergeStrategyConfig} from '../../config/index.js';
 import {PullRequest} from '../pull-request.js';
@@ -51,7 +52,7 @@ export class GithubApiMergeStrategy extends MergeStrategy {
    */
   override async merge(pullRequest: PullRequest): Promise<void> {
     const {githubTargetBranch, prNumber, needsCommitMessageFixup, targetBranches} = pullRequest;
-    const method = this.getMergeActionFromPullRequest(pullRequest);
+    const method = this._getMergeActionFromPullRequest(pullRequest);
     const cherryPickTargetBranches = targetBranches.filter((b) => b !== githubTargetBranch);
 
     const mergeOptions: OctokitMergeParams = {
@@ -194,7 +195,10 @@ export class GithubApiMergeStrategy extends MergeStrategy {
    * behavior here so that we have a default commit message that can be fixed up.
    */
   private async _getDefaultSquashCommitMessage(pullRequest: PullRequest): Promise<string> {
-    const commits = await this.getPullRequestCommits(pullRequest);
+    const commits = (await this._getPullRequestCommitMessages(pullRequest)).map((message) => ({
+      message,
+      parsed: parseCommitMessage(message),
+    }));
     const messageBase = `${pullRequest.title}${COMMIT_HEADER_SEPARATOR}`;
     if (commits.length <= 1) {
       return `${messageBase}${commits[0].parsed.body}`;
@@ -203,8 +207,17 @@ export class GithubApiMergeStrategy extends MergeStrategy {
     return `${messageBase}${joinedMessages}`;
   }
 
+  /** Gets all commit messages of commits in the pull request. */
+  private async _getPullRequestCommitMessages({prNumber}: PullRequest) {
+    const allCommits = await this.git.github.paginate(this.git.github.pulls.listCommits, {
+      ...this.git.remoteParams,
+      pull_number: prNumber,
+    });
+    return allCommits.map(({commit}) => commit.message);
+  }
+
   /** Determines the merge action from the given pull request. */
-  getMergeActionFromPullRequest({labels}: PullRequest): GithubApiMergeMethod {
+  private _getMergeActionFromPullRequest({labels}: PullRequest): GithubApiMergeMethod {
     if (this._config.labels) {
       const matchingLabel = this._config.labels.find(({pattern}) => labels.includes(pattern));
       if (matchingLabel !== undefined) {

ng-dev/pr/merge/strategies/conditional-autosquash-merge.ts

@@ -6,7 +6,6 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {Commit, parseCommitMessage} from '../../../commit-message/parse.js';
 import {AuthenticatedGitClient} from '../../../utils/git/authenticated-git-client.js';
 import {
   FatalMergeToolError,
@@ -201,22 +200,4 @@ export abstract class MergeStrategy {
       throw new MergeConflictsFatalError(failedBranches);
     }
   }
-
-  /** Gets all commit messages of commits in the pull request. */
-  protected async getPullRequestCommits({prNumber}: PullRequest): Promise<
-    {
-      message: string;
-      parsed: Commit;
-    }[]
-  > {
-    const allCommits = await this.git.github.paginate(this.git.github.pulls.listCommits, {
-      ...this.git.remoteParams,
-      pull_number: prNumber,
-    });
-
-    return allCommits.map(({commit: {message}}) => ({
-      message,
-      parsed: parseCommitMessage(message),
-    }));
-  }
 }