Skip to content

Commit 5232133

Browse files
evangraykfacebook-github-bot
authored andcommitted
Only check merge queue support once
Summary: Instead of caching the result of the promise, cache the promise itself. This means if we already have a check ongoing, we don't spawn a new call to `gh` to find if merge queue is supported. Reviewed By: muirdm Differential Revision: D76067307 fbshipit-source-id: 8d948ce2b69d059b1a5893c1b0df9cb4ea0158a0
1 parent 17aaf2c commit 5232133

1 file changed

Lines changed: 20 additions & 13 deletions

File tree

addons/isl-server/src/github/githubCodeReviewProvider.ts

Lines changed: 20 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ type GitHubCodeReviewSystem = CodeReviewSystem & {type: 'github'};
6666
export class GitHubCodeReviewProvider implements CodeReviewProvider {
6767
constructor(private codeReviewSystem: GitHubCodeReviewSystem, private logger: Logger) {}
6868
private diffSummaries = new TypedEventEmitter<'data', Map<DiffId, GitHubDiffSummary>>();
69-
private hasMergeQueueSupport: boolean | null = null;
69+
private hasMergeQueueSupport: Promise<boolean> | null = null;
7070

7171
onChangeDiffSummaries(
7272
callback: (result: Result<Map<DiffId, GitHubDiffSummary>>) => unknown,
@@ -83,12 +83,23 @@ export class GitHubCodeReviewProvider implements CodeReviewProvider {
8383
};
8484
}
8585

86-
private async detectMergeQueueSupport(): Promise<boolean> {
87-
const data = await this.query<MergeQueueSupportQueryData, MergeQueueSupportQueryVariables>(
88-
MergeQueueSupportQuery,
89-
{},
90-
);
91-
return data?.__type != null;
86+
private detectMergeQueueSupport(): Promise<boolean> {
87+
if (this.hasMergeQueueSupport == null) {
88+
this.hasMergeQueueSupport = (async (): Promise<boolean> => {
89+
this.logger.info('detecting if merge queue is supported');
90+
const data = await this.query<MergeQueueSupportQueryData, MergeQueueSupportQueryVariables>(
91+
MergeQueueSupportQuery,
92+
{},
93+
).catch(err => {
94+
this.logger.info('failed to detect merge queue support', err);
95+
return undefined;
96+
});
97+
const hasMergeQueueSupport = data?.__type != null;
98+
this.logger.info('set merge queue support to ' + hasMergeQueueSupport);
99+
return hasMergeQueueSupport;
100+
})();
101+
}
102+
return this.hasMergeQueueSupport;
92103
}
93104

94105
private fetchYourPullRequestsGraphQL(
@@ -117,13 +128,9 @@ export class GitHubCodeReviewProvider implements CodeReviewProvider {
117128
triggerDiffSummariesFetch = debounce(
118129
async () => {
119130
try {
120-
if (this.hasMergeQueueSupport == null) {
121-
this.logger.info('detecting if merge queue is supported');
122-
this.hasMergeQueueSupport = (await this.detectMergeQueueSupport()) ?? false;
123-
this.logger.info('set merge queue support to ' + this.hasMergeQueueSupport);
124-
}
131+
const hasMergeQueueSupport = await this.detectMergeQueueSupport();
125132
this.logger.info('fetching github PR summaries');
126-
const allSummaries = await this.fetchYourPullRequestsGraphQL(this.hasMergeQueueSupport);
133+
const allSummaries = await this.fetchYourPullRequestsGraphQL(hasMergeQueueSupport);
127134
if (allSummaries?.search.nodes == null) {
128135
this.diffSummaries.emit('data', new Map());
129136
return;

0 commit comments

Comments
 (0)