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

Add toJson() for events and get rid of CreateRoomJob::StateEvent #733

Open
KitsuneRal opened this issue Apr 4, 2024 · 0 comments
Open
Labels
enhancement A feature or change request for the library

Comments

@KitsuneRal
Copy link
Member

With all event classes ultimately being mere wrappers around QJsonObject, there's no particular reason we shouldn't enable their serialisation by simply calling Event::fullJson(). There used to be Event::toJson() before but it was deemed confusing as usually it was Event::contentJson() (that didn't exist back then) actually needed. That being said, we now have (automatically generated but no less superfluous) CreateRoomJob::StateEvent that duplicates StateEvent in terms of structure, except that it actually keeps the type, the state key, and the content separate. It can be argued whether this model might be better for all events (eliminating the need for Event::editJson() kludge?) but it certainly is duplication serving no good purpose.

@KitsuneRal KitsuneRal added the enhancement A feature or change request for the library label Apr 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement A feature or change request for the library
Projects
Status: 0.9 - To Do
Development

No branches or pull requests

1 participant