Skip to content

Lifecycle events#35

Merged
PenguinBoi12 merged 2 commits intomainfrom
lifecycle-events
Apr 6, 2026
Merged

Lifecycle events#35
PenguinBoi12 merged 2 commits intomainfrom
lifecycle-events

Conversation

@PenguinBoi12
Copy link
Copy Markdown
Contributor

Add a hook decorator and supporting infrastructure to register lifecycle event handlers (e.g. on_ready, on_error, on_command). Splits the existing _dispatch method into a generic lifecycle dispatcher and a Matrix-specific event dispatcher, and ensures hook handlers are properly propagated when loading extensions.

Multiple handlers per lifecycle event are supported and fired in registration order.

@PenguinBoi12 PenguinBoi12 force-pushed the lifecycle-events branch 2 times, most recently from f3a2a27 to f434b1b Compare April 4, 2026 22:17
@PenguinBoi12 PenguinBoi12 force-pushed the lifecycle-events branch 2 times, most recently from 93ad175 to f1c870d Compare April 5, 2026 07:50
@PenguinBoi12 PenguinBoi12 marked this pull request as ready for review April 5, 2026 08:52
Copy link
Copy Markdown
Contributor

@chrisdedman chrisdedman left a comment

Choose a reason for hiding this comment

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

Just 2 comments on comments in the code, otherwise LGTM

Comment thread matrix/bot.py
:type ctx: Context
"""
await self._process_commands(room, event)
# LIFECYCLE
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

what's this comments?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It was to organise/separate the code a bit better to make it easier to read.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do you think it would be worth separating those into files rather than keeping all in the same file?

Copy link
Copy Markdown
Contributor Author

@PenguinBoi12 PenguinBoi12 Apr 6, 2026

Choose a reason for hiding this comment

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

Good question, I thought about it but no, those belong here. They are part of the logic of the bot; it's the bot that need to handle events it's just that we have 2 types of events now - lifecycle events (our internal events) and matrix events (the events we receive from matrix). If you look you'll see that it's mostly event related methods. The only reason why I added the comment was to cleanly create a separation between our types of events. The rest is mostly small helpers related to events (so it's still belong there).

The only thing that might be moved away since it's more "client" related is the get_room. But that would require a wrapper around AsyncClient and it's out of scope for this PR. Plus, it's not super urgent since we only have this method for now.

Comment thread matrix/bot.py
def start(self) -> None:
"""
Synchronous entry point for running the bot.
# MATRIX EVENTS
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

can you explain the purpose of those comments?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Contributor

@chrisdedman chrisdedman left a comment

Choose a reason for hiding this comment

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

Great work. Thank you for the explanation. LGTM

@PenguinBoi12 PenguinBoi12 merged commit 9a7d62b into main Apr 6, 2026
4 checks passed
@PenguinBoi12 PenguinBoi12 deleted the lifecycle-events branch April 6, 2026 02:18
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.

2 participants