Skip to content

Commit 8c05b3a

Browse files
Don't try to pre-populate an octokit
I argue that calling createOctokit(false) adds no benefit. If an authenticated session already exists then this silently create an octokit, which makes getOctokit() a no-op just returning the field. However if there is already an authenticated session then getOctokit() would already be able to create an octokit without prompting the user. On the other hand if there isn't an authenticated session then we won't be able to pre-populate an octokit, so getOctokit() will have to prompt the user anyway. Not calling createOctokit(false) in registerListeners also doesn't change behaviour. If the user is authenticated in the new session then we would be able to create an octokit instance wihtout prompting in getOctokit anyway. If the user is not authenticated in the new session then we won't be able to create an instance without prompting either way. The only benefit I can think of is that it moves a tiny amount of computation earlier in the pipeline, but the amount of computation is tiny and it isn't any more async than it would be if it happened in getOctokit(). I don't think this is worth making the code more complex.
1 parent 74f10a3 commit 8c05b3a

1 file changed

Lines changed: 1 addition & 2 deletions

File tree

extensions/ql-vscode/src/authentication.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@ export class Credentials {
3434
): Promise<Credentials> {
3535
const c = new Credentials();
3636
c.registerListeners(context);
37-
c.octokit = await c.createOctokit(false);
3837
return c;
3938
}
4039

@@ -76,7 +75,7 @@ export class Credentials {
7675
context.subscriptions.push(
7776
vscode.authentication.onDidChangeSessions(async (e) => {
7877
if (e.provider.id === GITHUB_AUTH_PROVIDER_ID) {
79-
this.octokit = await this.createOctokit(false);
78+
this.octokit = undefined;
8079
}
8180
}),
8281
);

0 commit comments

Comments
 (0)