Skip to content

Commit

Permalink
Always auto-merge after creating and updating PRs
Browse files Browse the repository at this point in the history
  • Loading branch information
dgellow committed Nov 8, 2023
1 parent 76098bc commit 843fb5c
Show file tree
Hide file tree
Showing 2 changed files with 112 additions and 77 deletions.
132 changes: 60 additions & 72 deletions src/github.ts
Original file line number Diff line number Diff line change
Expand Up @@ -224,10 +224,6 @@ export type MergeMethod = 'merge' | 'squash' | 'rebase';
interface CreatePullRequestOptions {
fork?: boolean;
draft?: boolean;
reviewers?: string[];
autoMerge?: {
mergeMethod: MergeMethod;
};
/**
* If the number of an existing pull request is passed, its HEAD branch and attributes (title, labels, etc) will be
* updated instead of creating a new pull request.
Expand All @@ -238,10 +234,6 @@ interface CreatePullRequestOptions {
interface UpdatePullRequestOptions {
signoffUser?: string;
fork?: boolean;
reviewers?: string[];
autoMerge?: {
mergeMethod: MergeMethod;
};
pullRequestOverflowHandler?: PullRequestOverflowHandler;
}

Expand Down Expand Up @@ -1338,40 +1330,6 @@ export class GitHub {
pullRequest.labels
);

// attempt to enable auto-merge
let directlyMerged = false;
if (options?.autoMerge) {
try {
const result = await this.enablePullRequestAutoMerge(
pullRequestNumber,
options.autoMerge.mergeMethod
);
directlyMerged = result === 'direct-merged';
} catch (e: unknown) {
this.logger.error(
isOctokitGraphqlResponseError(e) ? e.errors || [] : (e as {}),
'Failed to enable auto merge. Continuing.'
);
}
}

// assign reviewers
if (!directlyMerged && options?.reviewers) {
try {
await this.octokit.pulls.requestReviewers({
owner: this.repository.owner,
repo: this.repository.repo,
pull_number: pullRequestNumber,
reviewers: options.reviewers,
});
} catch (error: unknown) {
this.logger.error(
isOctokitRequestError(error) ? error.message : (error as {}),
'Failed to add reviewers. Continuing.'
);
}
}

return await this.getPullRequest(pullRequestNumber);
}
);
Expand Down Expand Up @@ -1452,9 +1410,7 @@ export class GitHub {
releasePullRequest.updates,
{
fork: options?.fork,
reviewers: options?.reviewers,
existingPrNumber: number,
autoMerge: options?.autoMerge,
}
);

Expand Down Expand Up @@ -2399,42 +2355,74 @@ export class GitHub {
this.logger.debug({response}, 'mutatePullrequestEnableAutoMerge');
}

private async enablePullRequestAutoMerge(
async enablePullRequestAutoMerge(
pullRequestNumber: number,
mergeMethod: MergeMethod
): Promise<'auto-merged' | 'direct-merged'> {
this.logger.debug('Enable PR auto-merge');
const prId = await this.queryPullRequestId(pullRequestNumber);
if (!prId) {
throw new Error(`No id found for pull request ${pullRequestNumber}`);
}
): Promise<'auto-merged' | 'direct-merged' | 'none'> {
try {
await this.mutatePullRequestEnableAutoMerge(prId, mergeMethod);
return 'auto-merged';
this.logger.debug('Enable PR auto-merge');
const prId = await this.queryPullRequestId(pullRequestNumber);
if (!prId) {
throw new Error(`No id found for pull request ${pullRequestNumber}`);
}
try {
await this.mutatePullRequestEnableAutoMerge(prId, mergeMethod);
return 'auto-merged';
} catch (e: unknown) {
if (
isOctokitGraphqlResponseError(e) &&
(e.errors || []).find(
err =>
err.type === 'UNPROCESSABLE' &&
(err.message.includes('Pull request is in clean status') ||
err.message.includes(
'Protected branch rules not configured for this branch'
))
)
) {
this.logger.debug(
'PR can be merged directly, do it instead of via GitHub auto-merge'
);
await this.octokit.pulls.merge({
owner: this.repository.owner,
repo: this.repository.repo,
pull_number: pullRequestNumber,
merge_method: mergeMethod,
});
return 'direct-merged';
} else {
throw e;
}
}
} catch (e: unknown) {
if (
isOctokitGraphqlResponseError(e) &&
(e.errors || []).find(
err =>
err.type === 'UNPROCESSABLE' &&
(err.message.includes('Pull request is in clean status') ||
err.message.includes(
'Protected branch rules not configured for this branch'
))
)
) {
this.logger.debug(
'PR can be merged directly, do it instead of via GitHub auto-merge'
);
await this.octokit.pulls.merge({
this.logger.error(
isOctokitGraphqlResponseError(e) ? e.errors || [] : (e as {}),
'Failed to enable auto merge. Continuing.'
);
return 'none';
}
}

async addPullRequestReviewers({
pullRequestNumber,
reviewers,
}: {
pullRequestNumber: number;
reviewers: string[];
}) {
if (reviewers) {
try {
await this.octokit.pulls.requestReviewers({
owner: this.repository.owner,
repo: this.repository.repo,
pull_number: pullRequestNumber,
merge_method: mergeMethod,
reviewers,
});
return 'direct-merged';
} else {
throw e;
} catch (error: unknown) {
this.logger.error(
isOctokitRequestError(error) ? error.message : (error as {}),
'Failed to add reviewers. Continuing.'
);
}
}
}
Expand Down
57 changes: 52 additions & 5 deletions src/manifest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1099,11 +1099,26 @@ export class Manifest {
{
fork: this.fork,
draft: pullRequest.draft,
reviewers: this.reviewers,
autoMerge: this.pullRequestAutoMergeOption(pullRequest),
}
);

let directlyMerged = false;
const autoMerge = this.pullRequestAutoMergeOption(pullRequest);
if (autoMerge) {
const result = await this.github.enablePullRequestAutoMerge(
newPullRequest.number,
autoMerge.mergeMethod
);
directlyMerged = result === 'direct-merged';
}

if (!directlyMerged) {
this.github.addPullRequestReviewers({
pullRequestNumber: newPullRequest.number,
reviewers: this.reviewers,
});
}

return newPullRequest;
}

Expand All @@ -1130,13 +1145,28 @@ export class Manifest {
this.changesBranch,
{
fork: this.fork,
reviewers: this.reviewers,
signoffUser: this.signoffUser,
pullRequestOverflowHandler: this.pullRequestOverflowHandler,
autoMerge: this.pullRequestAutoMergeOption(pullRequest),
}
);

let directlyMerged = false;
const autoMerge = this.pullRequestAutoMergeOption(pullRequest);
if (autoMerge) {
const result = await this.github.enablePullRequestAutoMerge(
updatedPullRequest.number,
autoMerge.mergeMethod
);
directlyMerged = result === 'direct-merged';
}

if (!directlyMerged) {
this.github.addPullRequestReviewers({
pullRequestNumber: updatedPullRequest.number,
reviewers: this.reviewers,
});
}

return updatedPullRequest;
}

Expand All @@ -1161,11 +1191,28 @@ export class Manifest {
fork: this.fork,
signoffUser: this.signoffUser,
pullRequestOverflowHandler: this.pullRequestOverflowHandler,
autoMerge: this.pullRequestAutoMergeOption(pullRequest),
}
);
// TODO: consider leaving the snooze label
await this.github.removeIssueLabels([SNOOZE_LABEL], snoozed.number);

let directlyMerged = false;
const autoMerge = this.pullRequestAutoMergeOption(pullRequest);
if (autoMerge) {
const result = await this.github.enablePullRequestAutoMerge(
updatedPullRequest.number,
autoMerge.mergeMethod
);
directlyMerged = result === 'direct-merged';
}

if (!directlyMerged) {
this.github.addPullRequestReviewers({
pullRequestNumber: updatedPullRequest.number,
reviewers: this.reviewers,
});
}

return updatedPullRequest;
}

Expand Down

0 comments on commit 843fb5c

Please sign in to comment.