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

Migrate add_command function to send_message_to_user function #1912

Merged
merged 2 commits into from
May 8, 2022

Conversation

ALazyMeme
Copy link
Member

@ALazyMeme ALazyMeme commented May 7, 2022

Pull request checklist:

  • CHANGELOG.md was updated, if applicable
  • Documentation in docs/ or install-docs/ was updated, if applicable
  • I have tested all changes

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.

I'd prefer this sort of change was done one function at a time, with the function being moved out of dispatch.py - dispatch.py is not a concept that belongs in the current version of pajbot.

Overarching refactor/changes like this is also a lot more tedious to test because you have ~15-20 things to test every time - 10 small PRs > 1 big PR

@ALazyMeme ALazyMeme force-pushed the chore/dispatch-module-message-migration branch from 78282b5 to 033c201 Compare May 8, 2022 10:39
@ALazyMeme ALazyMeme changed the title Migrate dispatch module to send_message & send_message_to_user functions Migrate add_command function to send_message_to_user function May 8, 2022
@ALazyMeme ALazyMeme requested a review from pajlada May 8, 2022 10:42
@pajlada
Copy link
Member

pajlada commented May 8, 2022

Tested all branches, works 👍

@pajlada pajlada enabled auto-merge (squash) May 8, 2022 10:53
@pajlada pajlada merged commit 4e89884 into master May 8, 2022
@pajlada pajlada deleted the chore/dispatch-module-message-migration branch May 8, 2022 11:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants