Conversation
| return s | ||
|
|
||
|
|
||
| invite_message = _fix_markdown( |
There was a problem hiding this comment.
Could as well just have a Jinja template instead.
There was a problem hiding this comment.
That is a good point! I would like to add that in as well.
|
|
||
| from .db import PersistentStringSet | ||
| from .gh import GithubApp | ||
| from .events import github_app |
There was a problem hiding this comment.
It's not "events", it's
| from .events import github_app | |
| from .event_handlers import github_app |
conceptionally.
There was a problem hiding this comment.
I suppose we might want the name to include "github" too, since we might have other kinds of events later?
I also guess we might eventually want to refactor along the lines of logically-related-features, like "inviting new users", "dependency bumping", etc., instead of lower-level functional categories like "reacting to github events", "text strings", etc.
But no need to make the perfect the enemy of the good... I'm happy with merging this now and then worrying about that stuff later. @wgwz, what do you think?
There was a problem hiding this comment.
I like both of the suggestions! I definitely prefer taking it one step further and splitting up the modules into logically related features. I'm willing to do that for this MR.
Co-Authored-By: Sviatoslav Sydorenko <wk@sydorenko.org.ua>
3 participants
reading through the code i saw some opportunities to refactor. let me know what you think. the main idea is that i wanted to separate the quart app setup/controllers, from the github event handling code. i feel that it's a bit easier to read when it's split up this way. i got carried away and also decided it might be helpful to create a module that contains just messages that will get sent to users.