Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

feat: Stream announce backoff #146

Closed
wants to merge 2 commits into from
Closed

Conversation

Hona
Copy link
Collaborator

@Hona Hona commented Jan 31, 2023

Closes #145

Note - untested. Please do your due diligence, and note the config changes needed for prod

@Hona Hona marked this pull request as ready for review January 31, 2023 05:43
@Hona Hona requested a review from tsa96 January 31, 2023 05:46
Copy link
Member

@tsa96 tsa96 left a comment

Choose a reason for hiding this comment

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

Tried to test this and I'm afraid I can't get it working, if I had to guess it seems like _goneLiveEvents is being trimmed incorrectly, having stepped through in a debugger that list seemed to be empty during the checks on L171-L177.

I've spent almost an hour setting this up and testing at the point and I just don't think it's reasonable for the review to be to one trying to fix code that hasn't been tested by the submitter, so I'm going to have to ask you test this yourself. I can invite you to the shared bot testing server that you used to be in and give you my config.json file that's configured for it.

In terms of other comments, you should add the new config keys to config.template.json. Also, when testing I noticed we're constantly querying Discord to get the ID of the game on Twitch. This is unnecessary, we already know it (2043449207), let's remove that task and just get the ID from the config file.

@Gocnak
Copy link
Member

Gocnak commented Jun 1, 2023

Closing, needs more contributor testing. Feel free to re-open if you do so.

@Gocnak Gocnak closed this Jun 1, 2023
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.

Rate limit Twitch stream announces
3 participants