-
Notifications
You must be signed in to change notification settings - Fork 0
Refactor Message Handling and Reaction API #9
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
base: main
Are you sure you want to change the base?
Conversation
…mprove parameter handling
…d improve reaction handling
…y parameters in send methods
…tion. The reason is because we are using type hints which having it also in the docstring just duplicate unecessarly.
PenguinBoi12
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few small thing to change but otherwise good.
| :return: The constructed Message instance. | ||
| :raise MissingArgumentError: If event is None. | ||
| """ | ||
| if event is None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not necessary since we pass an event.
| :return: The constructed Message instance. | ||
| """ | ||
| if not event and not bot: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This shouldn't happen since we have bot and event as parameter
| """ | ||
| try: | ||
| msg = Message(self.bot) | ||
| msg = self.get_message(self.bot, event) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can probably get ride of get_message for now and use from_event directly
| event_id = event.event_id | ||
| body = event.body | ||
|
|
||
| return Message( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like we should pass the the event now
| if event is None: | ||
| raise ValueError("event cannot be None") | ||
|
|
||
| if isinstance(event, Event) and event.source["type"] == "m.reaction": |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't make sense either. It should always be an instance of event.cif they goal is to check the type just check the types you don't have to check the instance
Description
Refactored message and reaction handling to simplify the API and clean up reaction sending. Updated examples and internal methods to match the new interface.