-
Notifications
You must be signed in to change notification settings - Fork 362
feat(ffi): Add fn RoomPowerLevels::events #5937
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
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5937 +/- ##
=======================================
Coverage 88.54% 88.55%
=======================================
Files 363 363
Lines 104020 104020
Branches 104020 104020
=======================================
+ Hits 92106 92112 +6
+ Misses 7537 7530 -7
- Partials 4377 4378 +1 ☔ View full report in Codecov by Sentry. |
CodSpeed Performance ReportMerging #5937 will not alter performanceComparing Summary
|
76067db to
bda5084
Compare
|
FWIW all of the refactoring was done so we don't pass a raw string around for the event types in the FFI layer. If we think this is an overkill we can just keep the first commit and ignore the rest, although I think the new changes make more sense. |
|
It seems like #5931 it's doing something quite similar, even providing extra functionality on some parts 🤔 . However, there we now have |
poljar
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.
Looks good, please just add a changelog entry and docs on the method.
| fn events(&self) -> HashMap<crate::event::TimelineEventType, i64> { | ||
| self.inner.events.iter().map(|(key, value)| (key.clone().into(), (*value).into())).collect() | ||
| } |
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.
Docs please, also it's a bit weird that this isn't pub when all of our other exported methods are.
|
@poljar I finally got the CC tests to pass in matrix-org/complement-crypto#219. Could you also approve those? |
With this we can query the power level value for any event type
…ince it also contains the event contents for some of the types
…type Use that for `RoomPowerLevels::events` instead.
ead5621 to
a10e290
Compare
a10e290 to
fa5f499
Compare
Changes:
Rename
ffi::TimelineEventTypetoffi::TimelineEventContentsince it had the contents inside the enum, not just the type.Create a new
ffi::TimelineEventTypeenum with just the type info. Add missing cases like the multimedia ones. For this I had to add new features to the Ruma dependency.Rename
ffi::TimelineEvent::event_typetoffi::TimelineEvent::contentto match the renaming done above.Add
ffi::RoomPowerLevels::eventsfunction, which returns aHashMap<TimelineEventType, i64>. All the previous changes are so we can use something type-safe for the keys, and also to fix the event type not containing only the type issue.Public API changes documented in changelogs (optional)
Signed-off-by: