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

TMI Rates #878

Merged
merged 38 commits into from Jun 7, 2020
Merged

TMI Rates #878

merged 38 commits into from Jun 7, 2020

Conversation

TroyKomodo
Copy link
Contributor

@TroyKomodo TroyKomodo commented May 18, 2020

Pull request checklist:

  • CHANGELOG.md was updated, if applicable
  • Documentation in docs/ or install-docs/ was updated, if applicable

Fixes #691

@TroyKomodo
Copy link
Contributor Author

@ALazyMeme had the known bot idea

Copy link
Contributor

@RAnders00 RAnders00 left a comment

Choose a reason for hiding this comment

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

There's a lot of changes I've requested. It might even be smarter to re-design the rate limiting in the bot altogether since as you can see it's not really flexible. In my opinion rate-limiting should be incorporated into bot.py (not one of the IRC classes) and all the logic should be "capsuled" into its own class.

configs/example.ini Outdated Show resolved Hide resolved
pajbot/bot.py Show resolved Hide resolved
configs/example.ini Outdated Show resolved Hide resolved
pajbot/bot.py Outdated Show resolved Hide resolved
pajbot/managers/connection.py Outdated Show resolved Hide resolved
pajbot/managers/connection.py Outdated Show resolved Hide resolved
pajbot/managers/connection.py Show resolved Hide resolved
pajbot/managers/connection.py Outdated Show resolved Hide resolved
pajbot/tmi.py Show resolved Hide resolved
pajbot/managers/connection.py Show resolved Hide resolved
TroyKomodo and others added 6 commits May 19, 2020 18:29
@TroyKomodo
Copy link
Contributor Author

Is there more i must fix/add?

@RAnders00
Copy link
Contributor

I think I want @pajlada to review this and work on it

@TroyKomodo
Copy link
Contributor Author

Sure.

Copy link
Member

@pajlada pajlada left a comment

Choose a reason for hiding this comment

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

Good change, fits in with the current way we do things. It may not be the optimal way of applying rate limits, but this is good for me 👍

I would like a "Major" changelog entry added considering it's a pretty big change.

pajbot/bot.py Show resolved Hide resolved
Copy link
Contributor

@RAnders00 RAnders00 left a comment

Choose a reason for hiding this comment

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

only changelog missing, as pajlada said

@TroyKomodo
Copy link
Contributor Author

OkayChamp I'll make the entry

@TroyKomodo
Copy link
Contributor Author

i did these commits via the web interface WAYTOODANK

@pajlada pajlada requested a review from RAnders00 May 31, 2020 09:32
@TroyKomodo
Copy link
Contributor Author

Is there more to change for this?

@RAnders00 RAnders00 requested a review from pajlada June 7, 2020 09:26
pajbot/tmi.py Outdated Show resolved Hide resolved
pajbot/tmi.py Outdated Show resolved Hide resolved
pajbot/tmi.py Outdated Show resolved Hide resolved
Co-authored-by: Ruben Anders <ruben.anders@robotty.de>
Copy link
Member

@pajlada pajlada left a comment

Choose a reason for hiding this comment

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

👍

configs/example.ini Outdated Show resolved Hide resolved
Copy link
Member

@pajlada pajlada left a comment

Choose a reason for hiding this comment

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

Please verify that whisper_output_mode set to chat works as expected

@pajlada pajlada merged commit e1627a1 into pajbot:master Jun 7, 2020
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.

verified = 1 flag should do more
3 participants