Skip to content
New issue

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

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

Already on GitHub? # to your account

fix: use throttling-wired octokit everywhere #291

Merged
merged 3 commits into from
May 9, 2023

Conversation

varl
Copy link
Contributor

@varl varl commented May 9, 2023

Related to #192.

This adds the throttling plugin to other usages of octokit that were overlooked the first time around.

cc: @Andarist

@changeset-bot
Copy link

changeset-bot bot commented May 9, 2023

🦋 Changeset detected

Latest commit: 3ea6ad4

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@changesets/action Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@varl
Copy link
Contributor Author

varl commented May 9, 2023

Ugh, test broke because the mock for getOctokit isn't used. Looking into a fix.

@@ -26,6 +26,40 @@ import type {} from "@octokit/plugin-throttling/dist-types/types.d";
// To avoid that, we ensure to cap the message to 60k chars.
const MAX_CHARACTERS_PER_MESSAGE = 60000;

const setupOctokit = (githubToken) => {
Copy link
Member

Choose a reason for hiding this comment

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

oh, now it makes sense... I totally missed the fact that each command~ has its own octokit instance

@Andarist
Copy link
Member

Andarist commented May 9, 2023

@varl let me know if you need help with that

@varl
Copy link
Contributor Author

varl commented May 9, 2023

@Andarist cheers, test fixed in e70152d. If you know of a better way, feel free to change it.

Since github.getOctokit isn't used anymore, I refactored the type to GitHub from @actions/github/lib/utils as used here: https://github.com/actions/toolkit/blob/main/packages/github/src/github.ts#L19

Hope that works the same way as before since it's the same instance type.

@varl varl requested a review from Andarist May 9, 2023 15:00
@Andarist Andarist merged commit db8a109 into changesets:main May 9, 2023
@github-actions github-actions bot mentioned this pull request May 9, 2023
@Andarist
Copy link
Member

Andarist commented May 9, 2023

@varl thank you for your work! I'll release this tomorrow since I just swapped the build system for a new one and I'd prefer not to release it while I'm stepping away from the computer after work 😅

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants