Skip to content

Upgrade to 18.0.1 octokit rest - #2017

Merged
Alex Ross (alexr00) merged 12 commits into
masterfrom
test/issue#2008
Jul 23, 2020
Merged

Upgrade to 18.0.1 octokit rest#2017
Alex Ross (alexr00) merged 12 commits into
masterfrom
test/issue#2008

Conversation

@alexr00

@alexr00 Alex Ross (alexr00) commented Jul 22, 2020

Copy link
Copy Markdown
Member

Fixes #2008

@alexr00 Alex Ross (alexr00) changed the title Upgrate to 18.0.1 octokit rest Upgrade to 18.0.1 octokit rest Jul 22, 2020
@alexr00
Alex Ross (alexr00) marked this pull request as ready for review July 22, 2020 14:53
@alexr00

Copy link
Copy Markdown
Member Author

Everything that I tested is working, so I think that we're good to merge after review.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good overall!

Comment thread src/github/credentials.ts Outdated
export interface AnnotatedOctokit extends Octokit {
currentUser?: Octokit.PullsGetResponseUser;
export interface AnnotatedOctokit {
octokit: Octokit;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of pulling this property in, what if we exposed currentUser on the GitHub interface below, making it a required property and setting it as part of createHub?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!


private isWebviewReady = false;
protected async _postMessage(message: any) {
// Without the following ready check, we can end up in a state where the message handler in the webview

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Made it even better ( no setInterval).

@alexr00

Copy link
Copy Markdown
Member Author

Thanks for reviewing!

@alexr00
Alex Ross (alexr00) merged commit e9eaebb into master Jul 23, 2020
@RMacfarlane
Rachel Macfarlane (RMacfarlane) deleted the test/issue#2008 branch November 12, 2020 16:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Update to latest @octokit/rest^18

2 participants