FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

refactor: singleton `octokit` instance for shared throttling state by gr2m · Pull Request #640 · semantic-release/github · GitHub

  1. semantic-release
  2. github

Repository navigation

refactor: singleton octokit instance for shared throttling state - #640

Draft
gr2m wants to merge 4 commits into
masterfrom
beta-singleton-octokit
ConversationCommits (4)ChecksFiles changed
Draft

refactor: singleton octokit instance for shared throttling state#640
gr2m wants to merge 4 commits into
masterfrom
beta-singleton-octokit

Conversation

gr2m commented May 29, 2023 •
edited by hearts Bot
Loading

Copy link
Copy Markdown
Member

gr2m force-pushed the beta-singleton-octokit branch from e7f4f8f to 15a127b Compare May 29, 2023 23:49
gr2m force-pushed the beta-singleton-octokit branch from 15a127b to 0893bef Compare May 30, 2023 00:02
gr2m changed the base branch from beta to beta-no-more-serial May 30, 2023 00:03
gr2m changed the title WIP singleton octokit instance refactor: singleton octokit instance for shared throttling state May 30, 2023
gr2m changed the title refactor: singleton octokit instance for shared throttling state refactor: singleton octokit instance for shared throttling state May 30, 2023
Base automatically changed from beta-no-more-serial to beta May 30, 2023 02:53
travi reviewed Jun 2, 2023
View reviewed changes

travi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

i like where this is headed. only comments are calling out most of your todos. am i correct to assume that is most of what remains for this to still be draft, or do you have additional plans?

Comment thread test/add-channel.test.js
});

test.serial("Update a release with a custom github url", async (t) => {
// TODO: move to integration tests

travi Jun 2, 2023 •
edited by hearts Bot
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

is this a change that you are wanting to include in this PR?

Comment thread test/publish.test.js
});

test("Publish a release with one asset and custom github url", async (t) => {
// TODO: move to integration tests

travi Jun 2, 2023 •
edited by hearts Bot
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

is this a change you are wanting to include in this PR?

Comment thread test/publish.test.js
});

test("Publish a release when env.GITHUB_URL is set to https://github.com (Default in GitHub Actions, #268)", async (t) => {
// TODO: move to integration test

travi Jun 2, 2023 •
edited by hearts Bot
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

is this a change you are wanting to include in this PR?

Comment thread test/success.test.js
t.true(fetch.done());
});

// TODO: move to integration test

travi Jun 2, 2023 •
edited by hearts Bot
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

is this a change you are wanting to include in this PR?

travi commented Jun 2, 2023 •
edited by hearts Bot
Loading

Copy link
Copy Markdown
Member

are we wanting to include this refactor in the ESM change, or follow up with it afterward? i've had no problems in my testing with the current beta that includes the ESM change for our two candidate plugins, so wondering if we promote those candidates or wait for this change to go in

gr2m commented Jun 2, 2023 •
edited by hearts Bot
Loading

Copy link
Copy Markdown
Member Author

only comments are calling out most of your todos. am i correct to assume that is most of what remains for this to still be draft

correct

are we wanting to include this refactor in the ESM change, or follow up with it afterward

I planned to give this another go today but I won't have time. I'd say let's go ahead with the ESM change and then do this afterwards.

Base automatically changed from beta to master June 2, 2023 19:15
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

travi travi left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

Back | FazBrowse Home | New Git URL