Skip to content

fix(ng-dev/pr): Move the cleanup of the merge attempt to the finally block #216

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

fix(ng-dev/pr): Move the cleanup of the merge attempt to the finally block #216

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 6 additions & 14 deletions ng-dev/pr/merge/task.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,13 +130,11 @@ export class PullRequestMergeTask {

// Branch or revision that is currently checked out so that we can switch back to
// it once the pull request has been merged.
let previousBranchOrRevision: null | string = null;
const previousBranchOrRevision = this.git.getCurrentBranchOrRevision();

// The following block runs Git commands as child processes. These Git commands can fail.
// We want to capture these command errors and return an appropriate merge request status.
try {
previousBranchOrRevision = this.git.getCurrentBranchOrRevision();

// Run preparations for the merge (e.g. fetching branches).
await strategy.prepare(pullRequest);

Expand All @@ -146,12 +144,6 @@ export class PullRequestMergeTask {
return {status: MergeStatus.FAILED, failure};
}

// Switch back to the previous branch. We need to do this before deleting the temporary
// branches because we cannot delete branches which are currently checked out.
this.git.run(['checkout', '-f', previousBranchOrRevision]);

await strategy.cleanup(pullRequest);

// Return a successful merge status.
return {status: MergeStatus.SUCCESS};
} catch (e) {
Expand All @@ -162,11 +154,11 @@ export class PullRequestMergeTask {
}
throw e;
} finally {
// Always try to restore the branch if possible. We don't want to leave
// the repository in a different state than before.
if (previousBranchOrRevision !== null) {
this.git.runGraceful(['checkout', '-f', previousBranchOrRevision]);
}
// Switch back to the previous branch. We need to do this before deleting the temporary
// branches because we cannot delete branches which are currently checked out.
this.git.run(['checkout', '-f', previousBranchOrRevision]);

await strategy.cleanup(pullRequest);
}
}
}