feat(dev-infra): support for caretaker note label in merge script (#37595)

Adds support for a caretaker note label to the merge script.
Whenever a configured label is applied, the merge script will
not merge automatically, but instead prompt first in order
to ensure that the caretaker paid attention to the manual
caretaker note on the PR. This helps if a PR needs special
attention.

PR Close #37595
This commit is contained in:
Paul Gschwendtner 2020-06-16 00:20:36 +02:00 committed by Misko Hevery
parent f5e39999d7
commit 8154bbd538
5 changed files with 41 additions and 2 deletions

View File

@ -53,6 +53,8 @@ export interface MergeConfig {
claSignedLabel: string|RegExp; claSignedLabel: string|RegExp;
/** Pattern that matches labels which imply a merge ready pull request. */ /** Pattern that matches labels which imply a merge ready pull request. */
mergeReadyLabel: string|RegExp; mergeReadyLabel: string|RegExp;
/** Label that is applied when special attention from the caretaker is required. */
caretakerNoteLabel?: string|RegExp;
/** Label which can be applied to fixup commit messages in the merge script. */ /** Label which can be applied to fixup commit messages in the merge script. */
commitMessageFixupLabel: string|RegExp; commitMessageFixupLabel: string|RegExp;
/** /**

View File

@ -90,7 +90,7 @@ export async function mergePullRequest(
/** /**
* Handles the merge result by printing console messages, exiting the process * Handles the merge result by printing console messages, exiting the process
* based on the result, or by restarting the merge if force mode has been enabled. * based on the result, or by restarting the merge if force mode has been enabled.
* @returns Whether the merge was successful or not. * @returns Whether the merge completed without errors or not.
*/ */
async function handleMergeResult(result: MergeResult, disableForceMergePrompt = false) { async function handleMergeResult(result: MergeResult, disableForceMergePrompt = false) {
const {failure, status} = result; const {failure, status} = result;
@ -98,7 +98,7 @@ export async function mergePullRequest(
switch (status) { switch (status) {
case MergeStatus.SUCCESS: case MergeStatus.SUCCESS:
info(green(`Successfully merged the pull request: ${prNumber}`)); info(green(`Successfully merged the pull request: #${prNumber}`));
return true; return true;
case MergeStatus.DIRTY_WORKING_DIR: case MergeStatus.DIRTY_WORKING_DIR:
error( error(
@ -114,6 +114,9 @@ export async function mergePullRequest(
error(red('An error related to interacting with Github has been discovered.')); error(red('An error related to interacting with Github has been discovered.'));
error(failure!.message); error(failure!.message);
return false; return false;
case MergeStatus.USER_ABORTED:
info(`Merge of pull request has been aborted manually: #${prNumber}`);
return true;
case MergeStatus.FAILED: case MergeStatus.FAILED:
error(yellow(`Could not merge the specified pull request.`)); error(yellow(`Could not merge the specified pull request.`));
error(red(failure!.message)); error(red(failure!.message));

View File

@ -0,0 +1,15 @@
/**
* @license
* Copyright Google LLC All Rights Reserved.
*
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license
*/
import {red} from '../../utils/console';
import {PullRequest} from './pull-request';
export function getCaretakerNotePromptMessage(pullRequest: PullRequest): string {
return red('Pull request has a caretaker note applied. Please make sure you read it.') +
`\nQuick link to PR: ${pullRequest.url}`;
}

View File

@ -17,6 +17,8 @@ import {PullRequestMergeTask} from './task';
/** Interface that describes a pull request. */ /** Interface that describes a pull request. */
export interface PullRequest { export interface PullRequest {
/** URL to the pull request. */
url: string;
/** Number of the pull request. */ /** Number of the pull request. */
prNumber: number; prNumber: number;
/** Title of the pull request. */ /** Title of the pull request. */
@ -33,6 +35,8 @@ export interface PullRequest {
requiredBaseSha?: string; requiredBaseSha?: string;
/** Whether the pull request commit message fixup. */ /** Whether the pull request commit message fixup. */
needsCommitMessageFixup: boolean; needsCommitMessageFixup: boolean;
/** Whether the pull request has a caretaker note. */
hasCaretakerNote: boolean;
} }
/** /**
@ -77,13 +81,17 @@ export async function loadAndValidatePullRequest(
config.requiredBaseCommits && config.requiredBaseCommits[githubTargetBranch]; config.requiredBaseCommits && config.requiredBaseCommits[githubTargetBranch];
const needsCommitMessageFixup = !!config.commitMessageFixupLabel && const needsCommitMessageFixup = !!config.commitMessageFixupLabel &&
labels.some(name => matchesPattern(name, config.commitMessageFixupLabel)); labels.some(name => matchesPattern(name, config.commitMessageFixupLabel));
const hasCaretakerNote = !!config.caretakerNoteLabel &&
labels.some(name => matchesPattern(name, config.caretakerNoteLabel!));
return { return {
url: prData.html_url,
prNumber, prNumber,
labels, labels,
requiredBaseSha, requiredBaseSha,
githubTargetBranch, githubTargetBranch,
needsCommitMessageFixup, needsCommitMessageFixup,
hasCaretakerNote,
title: prData.title, title: prData.title,
targetBranches: getBranchesFromTargetLabel(targetLabel, githubTargetBranch), targetBranches: getBranchesFromTargetLabel(targetLabel, githubTargetBranch),
commitCount: prData.commits, commitCount: prData.commits,

View File

@ -6,10 +6,12 @@
* found in the LICENSE file at https://angular.io/license * found in the LICENSE file at https://angular.io/license
*/ */
import {promptConfirm} from '../../utils/console';
import {GitClient, GitCommandError} from '../../utils/git'; import {GitClient, GitCommandError} from '../../utils/git';
import {MergeConfigWithRemote} from './config'; import {MergeConfigWithRemote} from './config';
import {PullRequestFailure} from './failures'; import {PullRequestFailure} from './failures';
import {getCaretakerNotePromptMessage} from './messages';
import {isPullRequest, loadAndValidatePullRequest,} from './pull-request'; import {isPullRequest, loadAndValidatePullRequest,} from './pull-request';
import {GithubApiMergeStrategy} from './strategies/api-merge'; import {GithubApiMergeStrategy} from './strategies/api-merge';
import {AutosquashMergeStrategy} from './strategies/autosquash-merge'; import {AutosquashMergeStrategy} from './strategies/autosquash-merge';
@ -23,6 +25,7 @@ export const enum MergeStatus {
DIRTY_WORKING_DIR, DIRTY_WORKING_DIR,
SUCCESS, SUCCESS,
FAILED, FAILED,
USER_ABORTED,
GITHUB_ERROR, GITHUB_ERROR,
} }
@ -72,6 +75,14 @@ export class PullRequestMergeTask {
return {status: MergeStatus.FAILED, failure: pullRequest}; return {status: MergeStatus.FAILED, failure: pullRequest};
} }
// If the pull request has a caretaker note applied, raise awareness by prompting
// the caretaker. The caretaker can then decide to proceed or abort the merge.
if (pullRequest.hasCaretakerNote &&
!await promptConfirm(
getCaretakerNotePromptMessage(pullRequest) + `\nDo you want to proceed merging?`)) {
return {status: MergeStatus.USER_ABORTED};
}
const strategy = this.config.githubApiMerge ? const strategy = this.config.githubApiMerge ?
new GithubApiMergeStrategy(this.git, this.config.githubApiMerge) : new GithubApiMergeStrategy(this.git, this.config.githubApiMerge) :
new AutosquashMergeStrategy(this.git); new AutosquashMergeStrategy(this.git);